Skip to content

refactor: share exporter fallback extraction#293

Open
mnajafian-nv wants to merge 2 commits into
NVIDIA:mainfrom
mnajafian-nv:refactor/shared-exporter-fallbacks
Open

refactor: share exporter fallback extraction#293
mnajafian-nv wants to merge 2 commits into
NVIDIA:mainfrom
mnajafian-nv:refactor/shared-exporter-fallbacks

Conversation

@mnajafian-nv

@mnajafian-nv mnajafian-nv commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Overview

Consolidates duplicated manual LLM fallback extraction used by the ATIF, OpenTelemetry, and OpenInference exporters while keeping exporter-specific projection and schema logic local.

This is intentionally scoped to observability exporter fallback readers. There is an open draft PR, #291, that touches a broader codec/provider-schema refactor and related observability files; this PR should be reviewed with that overlap in mind.

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

  • Added a shared observability helper for manual fallback extraction of usage fields, provider-reported cost, model names, reasoning fields, raw tool-call fields, and replay payload detection.
  • Updated ATIF, OpenTelemetry, and OpenInference exporters to consume the shared low-level readers.
  • Kept ATIF trajectory projection, OpenInference semantic attributes, total-token normalization, and cost projection local to each exporter.
  • Added focused helper coverage for precedence, currency filtering, amountless cost objects, provider aliases, tool-call aliases, and replay payload detection.

Overlap note:

Validation:

  • cargo fmt --all -- --check
  • cargo check -p nemo-relay --no-default-features --lib
  • targeted helper/exporter tests
  • just test-rust
  • uv run pre-commit run --all-files
  • explicit changed-file pre-commit pass

Where should the reviewer start?

Start with crates/core/src/observability/manual_llm_fallback.rs, then review the reduced call sites in atif.rs, otel.rs, and openinference.rs.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • N/A

Summary by CodeRabbit

Release Notes

  • Refactor
    • Centralized LLM observability extraction for usage, token counts, costs, model names, reasoning metadata, and tool-call details to improve consistency across provider formats.
    • Improved token and cached-token precedence and token normalization when provider fields differ.
    • Enhanced USD cost estimation and currency/component handling when cost details vary by payload shape.
    • Streamlined OpenInference replay and tool-call parsing by reusing standardized fallback logic.

@mnajafian-nv mnajafian-nv added this to the 0.5 milestone Jun 23, 2026
@mnajafian-nv mnajafian-nv self-assigned this Jun 23, 2026
@mnajafian-nv mnajafian-nv requested a review from a team as a code owner June 23, 2026 20:51
@github-actions github-actions Bot added size:XL PR is extra large Improvement improvement to existing functionality lang:rust PR changes/introduces Rust code labels Jun 23, 2026
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a893610b-3f46-424b-965a-411e934b60f9

📥 Commits

Reviewing files that changed from the base of the PR and between 594c82e and d29f0b1.

📒 Files selected for processing (5)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: WebAssembly / Test (windows-amd64)
  • GitHub Check: Node.js / Test (windows-arm64)
  • GitHub Check: WebAssembly / Test (windows-arm64)
  • GitHub Check: WebAssembly / Test (macos-arm64)
  • GitHub Check: Python / Test (windows-arm64)
  • GitHub Check: Rust / Test (windows-arm64)
  • GitHub Check: Rust / Test (windows-amd64)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Maintain documented and tested validation and report behavior for adaptive surfaces

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

crates/core/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/{core,adaptive}/**

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/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.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/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/core/tests/unit/atif_tests.rs
crates/core/src/observability/{atif,otel,openinference}.rs

📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)

When changing event fields in ATIF, OpenTelemetry, or OpenInference observability surfaces, keep the core event model in crates/core/src/observability/atif.rs, crates/core/src/observability/otel.rs, and crates/core/src/observability/openinference.rs in sync

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/openinference.rs
🛑 Comments failed to post (1)
crates/core/src/observability/openinference.rs (1)

1453-1464: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the shared tool-call name reader here.

This local helper now mirrors manual_tool_call_name, so future alias/precedence fixes can drift between display text and raw tool-call attributes.

Refactor to the shared helper
 fn tool_call_name(value: &Json) -> Option<String> {
-    value
-        .get("name")
-        .and_then(Json::as_str)
-        .or_else(|| value.get("toolName").and_then(Json::as_str))
-        .or_else(|| {
-            value
-                .get("function")
-                .and_then(|function| function.get("name"))
-                .and_then(Json::as_str)
-        })
-        .map(str::to_string)
+    manual_tool_call_name(value).map(str::to_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/observability/openinference.rs` around lines 1453 - 1464, The
tool_call_name function duplicates the logic that already exists in the
manual_tool_call_name shared helper. Refactor the tool_call_name function to
reuse the manual_tool_call_name helper instead of reimplementing the same name
extraction logic. This will ensure both functions stay in sync and prevent
future drift in how tool-call attributes are handled across display text and raw
tool-call data.
🔇 Additional comments (5)
crates/core/src/observability/manual_llm_fallback.rs (2)

21-29: LGTM!

Also applies to: 61-68, 104-107, 220-355, 486-529


131-156: 🎯 Functional Correctness

The concern described in this review comment does not apply to the current code. The helpers used by the OpenInference path are already properly gated:

  • manual_cost_total_for_currency_from_llm_output (line 146) has #[cfg(any(feature = "openinference", test))]
  • manual_cost_total_for_currency_from_maps (its dependency) also has #[cfg(any(feature = "openinference", test))]

OpenInference can successfully use these functions in an openinference-only build without requiring the otel feature. No code changes are needed.

crates/core/tests/unit/observability/openinference_tests.rs (1)

2113-2126: LGTM!

crates/core/src/observability/atif.rs (1)

699-725: LGTM!

Also applies to: 738-741, 788-809

crates/core/tests/unit/atif_tests.rs (1)

804-843: LGTM!

Also applies to: 927-945


Walkthrough

Introduces a new manual_llm_fallback module centralizing all JSON parsing helpers for LLM observability fields (token usage with alias and precedence logic, cost estimation for multiple shapes, model name, reasoning, tool-call fields, and OpenInference replay detection). Migrates atif.rs, openinference.rs, and otel.rs to delegate to these shared helpers, removing approximately 415 lines of duplicate inline parsing logic.

Changes

LLM Observability Parsing Centralization

Layer / File(s) Summary
New manual_llm_fallback module: types, readers, tests, registration
crates/core/src/observability/manual_llm_fallback.rs, crates/core/src/observability/mod.rs
Adds ManualUsageFields struct containing token count fields (prompt, completion, total, cache read/write) and ManualUsagePrecedence enum controlling search behavior across usage and token_usage maps. Implements usage readers with comprehensive alias-key support and nested cache-token-detail fallback, cost estimation for both cost_usd and cost object shapes with per-currency totals, model name extraction, reasoning effort/content readers, tool-call field extraction (id, name, arguments) across multiple key variants, and OpenInference replay payload/response detection with OpenClaw shape recognition via content.source prefix and placeholderRequest marker. Includes extensive unit tests covering aliasing, precedence modes, cost parsing with missing currency, reasoning/tool-call alias coverage, and replay detection. Module declared as pub(crate) in mod.rs.
Migrate openinference.rs to shared helpers
crates/core/src/observability/openinference.rs
Adds imports from manual_llm_fallback. Replaces inline OpenClaw replay detection with delegation to manual_replay_llm_payload and manual_replay_llm_response. Tool-call field extraction (id, name, arguments) is replaced with manual_tool_call_* helpers. USD cost computation, model name extraction, and usage token normalization are delegated to shared helpers. Removes now-redundant local helper functions. Adds test verifying tool-call top-level name field takes precedence in display text.
Migrate otel.rs to shared helpers
crates/core/src/observability/otel.rs
Adds imports for cost, usage, and model name extraction helpers. Replaces ~90 lines of manual LLM output parsing with thin wrappers delegating to manual_cost_estimate_from_llm_output, manual_usage_from_llm_output_with (using existing normalize_total_tokens), and manual_model_name_from_llm_output. Removes inline token/cost/usage extraction logic.
Migrate atif.rs to shared helpers
crates/core/src/observability/atif.rs, crates/core/tests/unit/atif_tests.rs
Adds imports from manual_llm_fallback. Updates extract_metrics to use manual_usage_fields_from_preferred_token_usage and manual_cost_estimate_from_usage, removing cost_usd_from_cost_object helper. Changes response_model_name to delegate to manual_model_name_from_llm_output. Updates reasoning extraction (extract_reasoning_effort, extract_reasoning_content) to delegate to manual_reasoning_* helpers. Refactors extract_tool_calls to use manual_tool_call_id, manual_tool_call_name, manual_tool_call_arguments. Adds test for cached_tokens field precedence across top-level, nested token-detail, and cache-related usage keys. Updates cost estimation test to assert cost_usd is None when cost object lacks required currency field.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.27% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits format with type 'refactor' and concisely summarizes the main change (consolidating shared fallback extraction logic).
Description check ✅ Passed Description covers all required sections: overview of consolidation, detailed changes, specific file guidance for reviewers, and acknowledgment of related work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/manual_llm_fallback.rs`:
- Around line 257-285: The parameter names usage and token_usage in both
first_u64_from_manual_usage and first_nested_u64_from_manual_usage are
semantically inverted compared to how callers pass them as primary and secondary
arguments, creating a misread hazard for future maintainers. Rename the usage
parameter to primary and the token_usage parameter to secondary in both
functions, then update all references to these parameters within the function
bodies to use the new names. This aligns the parameter names with the actual
semantic roles they serve at call sites.
🪄 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: 2969433a-1a06-41f4-aba2-89b31e90c363

📥 Commits

Reviewing files that changed from the base of the PR and between 67606b5 and 594c82e.

📒 Files selected for processing (5)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
  • GitHub Check: Go / Test (windows-arm64)
  • GitHub Check: Go / Test (windows-amd64)
  • GitHub Check: Go / Test (macos-arm64)
  • GitHub Check: Python / Test (windows-amd64)
  • GitHub Check: Node.js / Test (windows-amd64)
  • GitHub Check: Python / Test (windows-arm64)
  • GitHub Check: Node.js / Test (windows-arm64)
  • GitHub Check: Python / Test (macos-arm64)
  • GitHub Check: WebAssembly / Test (linux-arm64)
  • GitHub Check: Node.js / Test (macos-arm64)
  • GitHub Check: WebAssembly / Test (macos-arm64)
  • GitHub Check: WebAssembly / Test (windows-amd64)
  • GitHub Check: WebAssembly / Test (linux-amd64)
  • GitHub Check: Rust / Test (windows-amd64)
  • GitHub Check: WebAssembly / Test (windows-arm64)
  • GitHub Check: Rust / Test (macos-arm64)
  • GitHub Check: Rust / Test (windows-arm64)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

crates/core/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/{core,adaptive}/**

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/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/src/observability/mod.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • 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 event fields in ATIF, OpenTelemetry, or OpenInference observability surfaces, keep the core event model in crates/core/src/observability/atif.rs, crates/core/src/observability/otel.rs, and crates/core/src/observability/openinference.rs in sync

Files:

  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/openinference.rs
🔇 Additional comments (7)
crates/core/src/observability/mod.rs (2)

16-16: LGTM!


16-16: 📐 Maintainability & Code Quality | ⚡ Quick win

Run the full language matrix for this crates/core change.

This touches crates/core shared observability/runtime contract. The PR description only documents Rust validation (just test-rust, fmt, clippy, pre-commit). Per guidelines, crates/core changes require validating Python, Go, Node.js, and WebAssembly bindings as well, since they share the runtime contract.

As per coding guidelines, "If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly". As per path instructions, public behavior must be preserved across bindings.

Sources: Coding guidelines, Path instructions

crates/core/src/observability/manual_llm_fallback.rs (2)

219-255: LGTM!


373-570: LGTM!

crates/core/src/observability/openinference.rs (1)

35-39: LGTM!

Also applies to: 944-950, 1018-1018, 1030-1036, 1082-1084, 1111-1117, 1453-1455

crates/core/src/observability/otel.rs (1)

31-34: LGTM!

Also applies to: 714-724

crates/core/src/observability/atif.rs (1)

44-49: LGTM!

Also applies to: 699-742, 786-789, 860-860, 892-900, 1001-1007

Comment thread crates/core/src/observability/manual_llm_fallback.rs Outdated
@mnajafian-nv mnajafian-nv marked this pull request as draft June 23, 2026 21:01
@mnajafian-nv mnajafian-nv marked this pull request as ready for review June 23, 2026 21:29

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 1453-1464: The tool_call_name function duplicates the logic that
already exists in the manual_tool_call_name shared helper. Refactor the
tool_call_name function to reuse the manual_tool_call_name helper instead of
reimplementing the same name extraction logic. This will ensure both functions
stay in sync and prevent future drift in how tool-call attributes are handled
across display text and raw tool-call data.
🪄 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: a893610b-3f46-424b-965a-411e934b60f9

📥 Commits

Reviewing files that changed from the base of the PR and between 594c82e and d29f0b1.

📒 Files selected for processing (5)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: WebAssembly / Test (windows-amd64)
  • GitHub Check: Node.js / Test (windows-arm64)
  • GitHub Check: WebAssembly / Test (windows-arm64)
  • GitHub Check: WebAssembly / Test (macos-arm64)
  • GitHub Check: Python / Test (windows-arm64)
  • GitHub Check: Rust / Test (windows-arm64)
  • GitHub Check: Rust / Test (windows-amd64)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Maintain documented and tested validation and report behavior for adaptive surfaces

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

crates/core/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/{core,adaptive}/**

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/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.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/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/core/tests/unit/atif_tests.rs
crates/core/src/observability/{atif,otel,openinference}.rs

📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)

When changing event fields in ATIF, OpenTelemetry, or OpenInference observability surfaces, keep the core event model in crates/core/src/observability/atif.rs, crates/core/src/observability/otel.rs, and crates/core/src/observability/openinference.rs in sync

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/openinference.rs
🔇 Additional comments (5)
crates/core/src/observability/manual_llm_fallback.rs (2)

21-29: LGTM!

Also applies to: 61-68, 104-107, 220-355, 486-529


131-156: 🎯 Functional Correctness

The concern described in this review comment does not apply to the current code. The helpers used by the OpenInference path are already properly gated:

  • manual_cost_total_for_currency_from_llm_output (line 146) has #[cfg(any(feature = "openinference", test))]
  • manual_cost_total_for_currency_from_maps (its dependency) also has #[cfg(any(feature = "openinference", test))]

OpenInference can successfully use these functions in an openinference-only build without requiring the otel feature. No code changes are needed.

crates/core/tests/unit/observability/openinference_tests.rs (1)

2113-2126: LGTM!

crates/core/src/observability/atif.rs (1)

699-725: LGTM!

Also applies to: 738-741, 788-809

crates/core/tests/unit/atif_tests.rs (1)

804-843: LGTM!

Also applies to: 927-945

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

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 1453-1464: The tool_call_name function duplicates the logic that
already exists in the manual_tool_call_name shared helper. Refactor the
tool_call_name function to reuse the manual_tool_call_name helper instead of
reimplementing the same name extraction logic. This will ensure both functions
stay in sync and prevent future drift in how tool-call attributes are handled
across display text and raw tool-call data.
🪄 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: a893610b-3f46-424b-965a-411e934b60f9

📥 Commits

Reviewing files that changed from the base of the PR and between 594c82e and d29f0b1.

📒 Files selected for processing (5)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual_llm_fallback.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
📜 Review details
🔇 Additional comments (5)
crates/core/src/observability/manual_llm_fallback.rs (2)

21-29: LGTM!

Also applies to: 61-68, 104-107, 220-355, 486-529


131-156: 🎯 Functional Correctness

The concern described in this review comment does not apply to the current code. The helpers used by the OpenInference path are already properly gated:

  • manual_cost_total_for_currency_from_llm_output (line 146) has #[cfg(any(feature = "openinference", test))]
  • manual_cost_total_for_currency_from_maps (its dependency) also has #[cfg(any(feature = "openinference", test))]

OpenInference can successfully use these functions in an openinference-only build without requiring the otel feature. No code changes are needed.

crates/core/tests/unit/observability/openinference_tests.rs (1)

2113-2126: LGTM!

crates/core/src/observability/atif.rs (1)

699-725: LGTM!

Also applies to: 738-741, 788-809

crates/core/tests/unit/atif_tests.rs (1)

804-843: LGTM!

Also applies to: 927-945

🛑 Comments failed to post (1)
crates/core/src/observability/openinference.rs (1)

1453-1464: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the shared tool-call name reader here.

This local helper now mirrors manual_tool_call_name, so future alias/precedence fixes can drift between display text and raw tool-call attributes.

Refactor to the shared helper
 fn tool_call_name(value: &Json) -> Option<String> {
-    value
-        .get("name")
-        .and_then(Json::as_str)
-        .or_else(|| value.get("toolName").and_then(Json::as_str))
-        .or_else(|| {
-            value
-                .get("function")
-                .and_then(|function| function.get("name"))
-                .and_then(Json::as_str)
-        })
-        .map(str::to_string)
+    manual_tool_call_name(value).map(str::to_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/observability/openinference.rs` around lines 1453 - 1464, The
tool_call_name function duplicates the logic that already exists in the
manual_tool_call_name shared helper. Refactor the tool_call_name function to
reuse the manual_tool_call_name helper instead of reimplementing the same name
extraction logic. This will ensure both functions stay in sync and prevent
future drift in how tool-call attributes are handled across display text and raw
tool-call data.

@willkill07 willkill07 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best-effort codec-based approach in PR-291 is the same approach that should be applied here.

The intended purpose of the refactor is to avoid all of the specialized information extraction logic that we have. The various provider codecs are the "correct" and maintainable way to normalize this information consistently.

@mnajafian-nv

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. I’ll rework this toward the codec-based normalization path from PR #291.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Improvement improvement to existing functionality lang:rust PR changes/introduces Rust code size:XL PR is extra large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants