Skip to content

refactor: prefer normalized exporter fallbacks - #304

Merged
rapids-bot[bot] merged 7 commits into
NVIDIA:mainfrom
mnajafian-nv:refactor/exporter-fallback-parity
Jun 25, 2026
Merged

refactor: prefer normalized exporter fallbacks#304
rapids-bot[bot] merged 7 commits into
NVIDIA:mainfrom
mnajafian-nv:refactor/exporter-fallback-parity

Conversation

@mnajafian-nv

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

Copy link
Copy Markdown
Contributor

Overview

Refines exporter fallback handling so codec-normalized LLM response data is authoritative where available, while manual/raw parsing remains a narrow fallback for payloads that do not have normalized provider data.

  • 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

  • Uses normalized LLM response usage, cost, and model data before manual/raw fallback in ATIF, OpenTelemetry, and OpenInference paths.
  • Keeps shared manual fallback readers limited to scalar usage, cost, and model hints when normalized data is unavailable.
  • Removes shared raw tool-call field extraction; replay-shaped raw tool-call alias handling remains local to OpenInference because replay payloads do not have normalized provider response data.
  • Preserves exporter-specific projection, schema behavior, and cost policy boundaries:
    • OTel keeps non-USD cost reporting.
    • OpenInference keeps USD-only cost attributes.
    • ATIF keeps USD-only cost reporting and legacy cache-read precedence.
  • Adds regression coverage for annotated-vs-raw conflicts, model fallback, requested-vs-response model pricing, cost-policy boundaries, usage aliases, cache-read precedence, OpenInference replay aliases, and legacy display-name precedence.

Validation:

  • cargo fmt --all
  • git diff --check
  • cargo test -p nemo-relay observability::manual::tests -- --nocapture
  • cargo test -p nemo-relay observability::atif::tests -- --nocapture
  • cargo test -p nemo-relay observability::otel::tests -- --nocapture
  • cargo test -p nemo-relay observability::openinference::tests -- --nocapture
  • cargo check -p nemo-relay --no-default-features
  • cargo check -p nemo-relay --no-default-features --features otel
  • cargo check -p nemo-relay --no-default-features --features openinference
  • cargo clippy -p nemo-relay --all-targets -- -D warnings
  • PATH="$HOME/.local/nemo-relay-tools/bin:$PATH" uv run pre-commit run --all-files

Pre-commit was run with the repo-pinned cargo-about 0.8.4.

Where should the reviewer start?

Start with crates/core/src/observability/mod.rs for the shared model/cost selection helpers and crates/core/src/observability/manual.rs for the narrowed scalar fallback policies.

Then review the exporter call sites in atif.rs, otel.rs, and openinference.rs to see the normalized-first fallback order and exporter-local projection boundaries.

The most important tests are in manual_tests.rs, atif_tests.rs, openinference_tests.rs, and otel_tests.rs.

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

  • N/A

Summary by CodeRabbit

  • New Features
    • Improved LLM model-name and token/cost attribution using normalized end payloads and raw response model names.
    • Enhanced replay tool-call support with additional aliasing for tool IDs, names, and argument locations.
  • Bug Fixes
    • More accurate token and cost metrics when usage fields are mixed or partially normalized.
    • Refined manual cost extraction and currency handling via configurable cost policies (including USD-only behavior).
  • Tests
    • Updated and expanded coverage for pricing precedence, metrics merging, model-name selection, tool-call promotion, and manual cost policy rules.

Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
@mnajafian-nv mnajafian-nv self-assigned this Jun 25, 2026
@mnajafian-nv
mnajafian-nv requested a review from a team as a code owner June 25, 2026 15:50
@mnajafian-nv
mnajafian-nv marked this pull request as draft June 25, 2026 15:51
@github-actions github-actions Bot added the size:L PR is large label Jun 25, 2026
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Observability code now shares manual cost-policy parsing, response/model-based pricing selection, normalized tool-call extraction, and raw-event model-name derivation across ATIF, OpenInference, and OTEL paths. Tests were updated for cache precedence, pricing priority, tool-call aliases, and raw-model handling.

Changes

Shared observability parsing and pricing

Layer / File(s) Summary
Manual parsing and cost policy
crates/core/src/observability/mod.rs, crates/core/src/observability/manual.rs, crates/core/tests/unit/observability/manual_tests.rs
mod.rs un-gates manual and adds response/model cost selection; manual.rs adds ManualCostPolicy and reordered cache-read extraction; tests cover policy and cache precedence.
Shared pricing and model names
crates/core/src/observability/mod.rs, crates/core/src/observability/otel.rs, crates/core/src/observability/openinference.rs, crates/core/tests/unit/observability/otel_tests.rs, crates/core/tests/unit/observability/openinference_tests.rs
cost_from_llm_event and cost_total_from_llm_event switch to the shared manual cost policy and response/model fallback pricing, and span model names now come from model_name_for_llm_event; tests cover raw-model attributes and replay model-name handling.
ATIF metrics, pricing, and model names
crates/core/src/observability/atif.rs, crates/core/tests/unit/atif_tests.rs
extract_metrics merges normalized and manual usage, broadens known token keys, resolves costs through response/model pricing, and derives model names from model_name_for_llm_event; tests cover usage merging, pricing priority, and raw-model attribution.
Tool-call alias parsing
crates/core/src/observability/atif.rs, crates/core/src/observability/openinference.rs, crates/core/tests/unit/observability/openinference_tests.rs, crates/core/tests/unit/atif_tests.rs
Tool-call id, name, and argument extraction now accepts additional replay and legacy alias fields, and tests verify flattened alias output and legacy name precedence.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • NVIDIA/NeMo-Relay#300: Refactors the same response/model cost fallback path and updates observability cost extraction call sites.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the refactor.
Description check ✅ Passed The description matches the template with overview, details, reviewer start, and validation; the related issue field is the only incomplete part.
Docstring Coverage ✅ Passed Docstring coverage is 88.57% which is sufficient. The required threshold is 80.00%.
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 github-actions Bot added Improvement improvement to existing functionality lang:rust PR changes/introduces Rust code labels Jun 25, 2026
@mnajafian-nv mnajafian-nv added this to the 0.5 milestone Jun 25, 2026
@github-actions

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
@mnajafian-nv
mnajafian-nv marked this pull request as ready for review June 25, 2026 16:09

@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/tests/unit/observability/openinference_tests.rs`:
- Around line 1769-1781: The current test in display_text_from_tool_calls does
not exercise the intended precedence between toolName and function.name because
the first fixture includes name and short-circuits the fallback chain. Update
the openinference_tests case to add a fixture without name but with both
toolName and function.name populated, and assert the expected output so the
precedence behavior is explicitly pinned; keep the existing
display_text_from_tool_calls coverage for the other fallback cases.
🪄 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: 02876c85-4f2c-474b-b4eb-71f153c11c72

📥 Commits

Reviewing files that changed from the base of the PR and between 6fb3f42 and 00fa258.

📒 Files selected for processing (2)
  • crates/core/src/observability/openinference.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (26)
  • GitHub Check: Rust / Test (windows-arm64)
  • GitHub Check: Python / Test (macos-arm64)
  • GitHub Check: Node.js / Test (windows-arm64)
  • GitHub Check: Python / Test (windows-amd64)
  • GitHub Check: Rust / Test (windows-amd64)
  • GitHub Check: Python / Test (windows-arm64)
  • GitHub Check: Python / Test (linux-arm64)
  • GitHub Check: Python / Test (linux-amd64)
  • GitHub Check: Node.js / Test (macos-arm64)
  • GitHub Check: Node.js / Test (windows-amd64)
  • GitHub Check: Rust / Test (linux-arm64)
  • GitHub Check: Rust / Test (linux-amd64)
  • GitHub Check: WebAssembly / Test (windows-arm64)
  • GitHub Check: Rust / Test (macos-arm64)
  • GitHub Check: Node.js / Test (linux-arm64)
  • GitHub Check: WebAssembly / Test (windows-amd64)
  • GitHub Check: Go / Test (windows-arm64)
  • GitHub Check: Node.js / Test (linux-amd64)
  • GitHub Check: WebAssembly / Test (linux-arm64)
  • GitHub Check: Go / Test (windows-amd64)
  • GitHub Check: WebAssembly / Test (linux-amd64)
  • GitHub Check: Go / Test (linux-amd64)
  • GitHub Check: WebAssembly / Test (macos-arm64)
  • GitHub Check: Go / Test (linux-arm64)
  • GitHub Check: Go / Test (macos-arm64)
  • GitHub Check: Preview docs
🧰 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/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
**/{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/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/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/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/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/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/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/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/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/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/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/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/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/openinference.rs
🔇 Additional comments (1)
crates/core/src/observability/openinference.rs (1)

1531-1532: 📐 Maintainability & Code Quality

Please record the required crates/core validation matrix for this PR.

This change is in crates/core, but the supplied validation summary only mentions Rust-focused checks. Please add the required broader validation (validate-change plus the full binding matrix: just test-python, just test-node, just test-go, just test-wasm) or note where those already passed. As per coding guidelines, "crates/core/**/*.rs: If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation" and "crates/{core,adaptive}/**: If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly."

Source: Coding guidelines

Comment thread crates/core/tests/unit/observability/openinference_tests.rs Outdated

@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.

One of the primary goals of refactoring is avoid all of these or_else chains with using different keys and extraction methods.

We should be relying on the shape of AnnotatedLLMRequest and AnnotatedLLMResponse wherever possible.

Comment thread crates/core/src/observability/manual.rs Outdated
Comment thread crates/core/src/observability/atif.rs
Comment thread crates/core/src/observability/openinference.rs Outdated
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
@mnajafian-nv mnajafian-nv changed the title refactor: share manual LLM fallback readers refactor: prefer normalized exporter fallbacks Jun 25, 2026

@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: 4

🤖 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/atif.rs`:
- Around line 1024-1029: ATIF tool-call name extraction in the `name` assembly
path is missing the camel-case `toolName` alias, so alias-only calls are
skipped. Update the fallback chain in the tool-call parsing logic to check
`toolName` on the tool-call object before synthesizing the id, alongside the
existing `name`, `tool_name`, and `function_name` lookups. Keep the change
localized to the extraction code that builds `name` in `atif.rs`.
- Around line 1458-1465: The paired span metric extraction is using only the end
event’s model name, which can miss the requested model stored on the start
event. Update the logic around the end_metrics extraction in atif.rs to use the
paired start event as the fallback model source when calling extract_metrics,
while still preferring the end event’s model_name if present. Use the start/end
span pairing helpers and the existing end.data(), end.name(), and
extract_metrics flow to locate the fix.
- Around line 767-775: The extra field collection in AtifMetrics is only reading
from token_usage_object(output), so unknown keys present in usage are being lost
when merged_usage combines both maps. Update the extra_map construction in the
relevant AtifMetrics/merged_usage path to inspect both usage sources and
preserve any non-known fields from each before storing them in
AtifMetrics.extra. Use the existing symbols merged_usage, token_usage_object,
raw_usage, and extra_map to locate the merge logic and ensure extras from both
maps are retained.

In `@crates/core/src/observability/openinference.rs`:
- Around line 1130-1138: The raw tool-name lookup in raw_tool_call_name
currently prefers function.name first, which diverges from the legacy precedence
used elsewhere. Update raw_tool_call_name to match display_text_from_tool_calls
by checking top-level name, toolName, and tool_name before falling back to
function.name and function_name, so replay payloads resolve the same tool name
consistently across raw OpenInference attributes and display text.
🪄 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: e26ecf92-dd47-49e4-ae96-0cdb5c8fe9e2

📥 Commits

Reviewing files that changed from the base of the PR and between 00fa258 and 865379a.

📒 Files selected for processing (7)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/tests/unit/observability/manual_tests.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
💤 Files with no reviewable changes (2)
  • crates/core/src/observability/manual.rs
  • crates/core/tests/unit/observability/manual_tests.rs
📜 Review details
🧰 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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.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/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_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/tests/unit/observability/openinference_tests.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_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
  • 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/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/atif.rs
🔇 Additional comments (3)
crates/core/tests/unit/atif_tests.rs (1)

20-21: LGTM!

Also applies to: 103-130, 882-1375

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

1768-1783: LGTM!

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

718-746: LGTM!

@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: 4

🤖 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/atif.rs`:
- Around line 1024-1029: ATIF tool-call name extraction in the `name` assembly
path is missing the camel-case `toolName` alias, so alias-only calls are
skipped. Update the fallback chain in the tool-call parsing logic to check
`toolName` on the tool-call object before synthesizing the id, alongside the
existing `name`, `tool_name`, and `function_name` lookups. Keep the change
localized to the extraction code that builds `name` in `atif.rs`.
- Around line 1458-1465: The paired span metric extraction is using only the end
event’s model name, which can miss the requested model stored on the start
event. Update the logic around the end_metrics extraction in atif.rs to use the
paired start event as the fallback model source when calling extract_metrics,
while still preferring the end event’s model_name if present. Use the start/end
span pairing helpers and the existing end.data(), end.name(), and
extract_metrics flow to locate the fix.
- Around line 767-775: The extra field collection in AtifMetrics is only reading
from token_usage_object(output), so unknown keys present in usage are being lost
when merged_usage combines both maps. Update the extra_map construction in the
relevant AtifMetrics/merged_usage path to inspect both usage sources and
preserve any non-known fields from each before storing them in
AtifMetrics.extra. Use the existing symbols merged_usage, token_usage_object,
raw_usage, and extra_map to locate the merge logic and ensure extras from both
maps are retained.

In `@crates/core/src/observability/openinference.rs`:
- Around line 1130-1138: The raw tool-name lookup in raw_tool_call_name
currently prefers function.name first, which diverges from the legacy precedence
used elsewhere. Update raw_tool_call_name to match display_text_from_tool_calls
by checking top-level name, toolName, and tool_name before falling back to
function.name and function_name, so replay payloads resolve the same tool name
consistently across raw OpenInference attributes and display text.
🪄 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: e26ecf92-dd47-49e4-ae96-0cdb5c8fe9e2

📥 Commits

Reviewing files that changed from the base of the PR and between 00fa258 and 865379a.

📒 Files selected for processing (7)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/manual.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/tests/unit/atif_tests.rs
  • crates/core/tests/unit/observability/manual_tests.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
💤 Files with no reviewable changes (2)
  • crates/core/src/observability/manual.rs
  • crates/core/tests/unit/observability/manual_tests.rs
📜 Review details
🔇 Additional comments (3)
crates/core/tests/unit/atif_tests.rs (1)

20-21: LGTM!

Also applies to: 103-130, 882-1375

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

1768-1783: LGTM!

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

718-746: LGTM!

🛑 Comments failed to post (4)
crates/core/src/observability/atif.rs (3)

767-775: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Preserve extras from both usage maps.

merged_usage combines usage and token_usage, but extra_map only inspects token_usage_object(output). When both maps exist, unknown fields under usage are dropped from AtifMetrics.extra.

Proposed fix
-    let extra_map: serde_json::Map<String, Json> = raw_usage
-        .map(|usage| {
-            usage
-                .iter()
-                .filter(|(k, _)| !known.contains(k.as_str()))
-                .map(|(k, v)| (k.clone(), v.clone()))
-                .collect()
-        })
-        .unwrap_or_default();
+    let extra_map: serde_json::Map<String, Json> = output
+        .as_object()
+        .into_iter()
+        .flat_map(|output| {
+            ["usage", "token_usage"]
+                .into_iter()
+                .filter_map(|key| output.get(key).and_then(Json::as_object))
+        })
+        .flat_map(|usage| usage.iter())
+        .filter(|(k, _)| !known.contains(k.as_str()))
+        .map(|(k, v)| (k.clone(), v.clone()))
+        .collect();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    let extra_map: serde_json::Map<String, Json> = output
        .as_object()
        .into_iter()
        .flat_map(|output| {
            ["usage", "token_usage"]
                .into_iter()
                .filter_map(|key| output.get(key).and_then(Json::as_object))
        })
        .flat_map(|usage| usage.iter())
        .filter(|(k, _)| !known.contains(k.as_str()))
        .map(|(k, v)| (k.clone(), v.clone()))
        .collect();
🤖 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/atif.rs` around lines 767 - 775, The extra
field collection in AtifMetrics is only reading from token_usage_object(output),
so unknown keys present in usage are being lost when merged_usage combines both
maps. Update the extra_map construction in the relevant AtifMetrics/merged_usage
path to inspect both usage sources and preserve any non-known fields from each
before storing them in AtifMetrics.extra. Use the existing symbols merged_usage,
token_usage_object, raw_usage, and extra_map to locate the merge logic and
ensure extras from both maps are retained.

1024-1029: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle toolName in ATIF tool-call extraction.

OpenInference accepts the camel-case alias, but ATIF still skips alias-only calls because name remains empty. Add toolName before synthesizing the id.

Proposed fix
         let name = func
             .and_then(|f| f.get("name"))
             .or_else(|| tc_obj.get("name"))
+            .or_else(|| tc_obj.get("toolName"))
             .or_else(|| tc_obj.get("tool_name"))
             .or_else(|| tc_obj.get("function_name"))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        let name = func
            .and_then(|f| f.get("name"))
            .or_else(|| tc_obj.get("name"))
            .or_else(|| tc_obj.get("toolName"))
            .or_else(|| tc_obj.get("tool_name"))
            .or_else(|| tc_obj.get("function_name"))
            .and_then(Json::as_str)
🤖 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/atif.rs` around lines 1024 - 1029, ATIF
tool-call name extraction in the `name` assembly path is missing the camel-case
`toolName` alias, so alias-only calls are skipped. Update the fallback chain in
the tool-call parsing logic to check `toolName` on the tool-call object before
synthesizing the id, alongside the existing `name`, `tool_name`, and
`function_name` lookups. Keep the change localized to the extraction code that
builds `name` in `atif.rs`.

1458-1465: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the paired start event as the requested-model fallback.

For paired spans, the start event can carry the requested model while the end event only carries usage. Passing only end.model_name() makes pricing fall back to None even though start is available.

Proposed fix
             end_metrics: end.data().and_then(|output| {
                 let normalized_response = end.normalized_llm_response();
+                let requested_model = end.model_name().or_else(|| start.model_name());
                 extract_metrics(
                     output,
                     Some(end.name()),
-                    end.model_name(),
+                    requested_model,
                     normalized_response.as_deref(),
                 )
             }),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            end_metrics: end.data().and_then(|output| {
                let normalized_response = end.normalized_llm_response();
                let requested_model = end.model_name().or_else(|| start.model_name());
                extract_metrics(
                    output,
                    Some(end.name()),
                    requested_model,
                    normalized_response.as_deref(),
                )
🤖 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/atif.rs` around lines 1458 - 1465, The paired
span metric extraction is using only the end event’s model name, which can miss
the requested model stored on the start event. Update the logic around the
end_metrics extraction in atif.rs to use the paired start event as the fallback
model source when calling extract_metrics, while still preferring the end
event’s model_name if present. Use the start/end span pairing helpers and the
existing end.data(), end.name(), and extract_metrics flow to locate the fix.
crates/core/src/observability/openinference.rs (1)

1130-1138: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Match legacy raw tool-name precedence.

display_text_from_tool_calls preserves top-level/toolName precedence, but raw OpenInference attributes now prefer function.name. The same replay payload can export different names in display text vs function.name.

Proposed fix
 fn raw_tool_call_name(tool_call: &Json) -> Option<&str> {
     tool_call
-        .get("function")
-        .and_then(|function| function.get("name"))
-        .and_then(Json::as_str)
-        .or_else(|| tool_call.get("name").and_then(Json::as_str))
+        .get("name")
+        .and_then(Json::as_str)
         .or_else(|| tool_call.get("toolName").and_then(Json::as_str))
         .or_else(|| tool_call.get("tool_name").and_then(Json::as_str))
         .or_else(|| tool_call.get("function_name").and_then(Json::as_str))
+        .or_else(|| {
+            tool_call
+                .get("function")
+                .and_then(|function| function.get("name"))
+                .and_then(Json::as_str)
+        })
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

fn raw_tool_call_name(tool_call: &Json) -> Option<&str> {
    tool_call
        .get("name")
        .and_then(Json::as_str)
        .or_else(|| tool_call.get("toolName").and_then(Json::as_str))
        .or_else(|| tool_call.get("tool_name").and_then(Json::as_str))
        .or_else(|| tool_call.get("function_name").and_then(Json::as_str))
        .or_else(|| {
            tool_call
                .get("function")
                .and_then(|function| function.get("name"))
                .and_then(Json::as_str)
        })
🤖 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 1130 - 1138, The
raw tool-name lookup in raw_tool_call_name currently prefers function.name
first, which diverges from the legacy precedence used elsewhere. Update
raw_tool_call_name to match display_text_from_tool_calls by checking top-level
name, toolName, and tool_name before falling back to function.name and
function_name, so replay payloads resolve the same tool name consistently across
raw OpenInference attributes and display text.

@mnajafian-nv

Copy link
Copy Markdown
Contributor Author

Thanks. I reworked this so normalized/codec data is authoritative wherever available, and manual/raw extraction is only a narrow fallback.

The shared manual layer now covers scalar usage/cost/model hints only. I removed the shared raw tool-call reader and kept the remaining replay-shaped tool-call alias handling local to OpenInference.

I also updated the PR title/body and added coverage for normalized-first extraction, exporter cost-policy boundaries, and preserved local tool-call display behavior.

Comment thread crates/core/src/observability/atif.rs Outdated
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
@github-actions github-actions Bot added size:XL PR is extra large and removed size:L PR is large labels Jun 25, 2026

@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/atif.rs`:
- Around line 1455-1463: The LLM metrics path in the end-event handling is only
forwarding the end/model fallback from category-profile data into
extract_metrics, so pricing can miss the requested model when it is present only
on the start payload. Update the end_metrics extraction block to also pass the
start-event model fallback from start.model_name() (alongside end.model_name())
into extract_metrics, ensuring estimate_cost_for_response_or_model can price
deduped LLM metrics even when the response echo is unpriced.
🪄 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: a2172060-ee61-48c1-be91-2da366f8a7f5

📥 Commits

Reviewing files that changed from the base of the PR and between 865379a and 57ec4b5.

📒 Files selected for processing (5)
  • crates/core/src/observability/atif.rs
  • crates/core/src/observability/mod.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 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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/atif.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/openinference.rs
  • crates/core/src/observability/atif.rs
🔇 Additional comments (5)
crates/core/src/observability/mod.rs (1)

16-75: 📐 Maintainability & Code Quality

Confirm the required Rust/core validation was run.

This cohort changes crates/core Rust observability behavior; please confirm cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, just test-rust, and the affected full language matrix/pre-commit validation were run. As per coding guidelines, "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", and "Changes to crates/core or crates/adaptive must run the full language matrix."

Source: Coding guidelines

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

42-46: LGTM!

Also applies to: 704-797, 995-1042, 1313-1363, 2266-2328

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

972-999: LGTM!

Also applies to: 1248-1286, 2140-2164

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

25-25: LGTM!

Also applies to: 739-790, 1112-1125

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

1301-1383: LGTM!

Comment thread crates/core/src/observability/atif.rs

@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/atif.rs`:
- Around line 1457-1466: Update the ATIF metric fallback logic in the
`requested_model` construction so the paired start model is preferred for
pricing when available. In the `extract_metrics` paths around `requested_model`,
change the lookup order to use `start.model_name()` before `end.model_name()`,
while keeping the existing `model_name_for_llm_event(start)` fallback. Apply the
same adjustment in both affected metric paths to avoid using provider-echoed
aliases from `end` when the priced model is only present on `start`.
🪄 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: 04748943-2e86-4de7-80f5-4cd7b5eb2f05

📥 Commits

Reviewing files that changed from the base of the PR and between 57ec4b5 and 69e0de6.

📒 Files selected for processing (2)
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Check / Run
  • GitHub Check: Preview docs
🧰 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/atif_tests.rs
  • crates/core/src/observability/atif.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/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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/atif_tests.rs
  • crates/core/src/observability/atif.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/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
🔇 Additional comments (1)
crates/core/src/observability/atif.rs (1)

1343-1360: 📐 Maintainability & Code Quality

Run the required crates/core validation matrix for this revision.

This PR changes crates/core, so the repository guidance requires the Rust checks plus the full binding matrix. Please confirm this revision ran validate-change, uv run pre-commit run --all-files, just test-rust, just test-python, just test-go, just test-node, just test-wasm, cargo fmt --all, and cargo clippy --workspace --all-targets -- -D warnings. As per coding guidelines, "If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation" and "Changes to crates/core or crates/adaptive must run the full language matrix."

Source: Coding guidelines

Comment thread crates/core/src/observability/atif.rs Outdated
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
@zhongxuanwang-nv

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 1479945 into NVIDIA:main Jun 25, 2026
71 checks passed
@mnajafian-nv
mnajafian-nv deleted the refactor/exporter-fallback-parity branch June 25, 2026 19:55
zhongxuanwang-nv added a commit to zhongxuanwang-nv/NeMo-Relay that referenced this pull request Jun 29, 2026
Bring in NVIDIA#304 (prefer normalized exporter fallbacks) and NVIDIA#305 (shared relay DTO crate) so the token/cost docs and tests reflect current behavior. Clean auto-merge; my two contract tests pass on the merged tree.

Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
zhongxuanwang-nv added a commit to zhongxuanwang-nv/NeMo-Relay that referenced this pull request Jun 29, 2026
…ss review (RELAY-243)

After merging main, the docs predated NVIDIA#304 (prefer normalized exporter fallbacks) and NVIDIA#305 (shared DTO crate). Corrected: exporter cost precedence (normalized-first, umbrella wording); ATIF now consumes the codec annotation (not raw-only) in the exporter table, atif.mdx, and Stability; marked the ATIF raw-only divergence Fixed in 0.5 (known-issues), with USD-only cost kept as an active 0.5 limitation; qualified Granularity (user/agent steps; final_metrics sums present step metrics on this trajectory, excludes subagents); corrected the total_tokens projection note; scoped the additive-compat promise to serialized JSON/ATOF (Rust structs/enums are exhaustive); qualified 'optional/unset'.

Tests: rewrote the OTel cost-only test to drive OtelEventProcessor and assert the finished span's keys (rejecting any token-ish or gen_ai key, closing the nemo_relay.prompt_tokens gap); added a codec-level test that OpenAIChatCodec drops completion_tokens_details.
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
zhongxuanwang-nv added a commit to zhongxuanwang-nv/NeMo-Relay that referenced this pull request Jun 29, 2026
The ATIF raw-only behavior was never a documented limitation on main, and NVIDIA#304 already resolved it, so this token/cost-semantics PR should not introduce a release-notes 0.5 section or document another PR's fix. The USD-only cost behavior remains documented in the provider-response-codecs contract page (exporter table + Stability).

Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
rapids-bot Bot pushed a commit that referenced this pull request Jul 2, 2026
#### Overview

Backfill regression and parity coverage for the shared extraction seams established by #291, #300, #301, #304, and #318, so agent-payload extraction, provider-schema normalization, and exporter projection cannot drift silently. Test-only: the only `src` edits are `#[cfg(test)]` module wiring for two new suites; no production behavior changes.

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

#### Details

**Agent payload extraction** (`crates/cli/tests/coverage/adapters_tests.rs`, 15 tests):

- Per-host partial-sparse payloads for Claude Code, Codex, and Hermes (real identifiers kept, absent fields synthesized or null at the adapter boundary).
- Path-precedence fallback-chain walks for session IDs, event names, subagent IDs, tool-call IDs, and tool results/status.
- Hermes tool result and status extraction (previously uncovered), including explicit-status vs event-name-derived status interaction.
- Claude Code LLM-hint extraction and hint field precedence chains.
- JSON-path primitive edge cases: empty-string filtering, array intermediates, deep nesting, all-paths-miss.

**Provider request extraction** (`crates/cli/tests/coverage/alignment_tests.rs`, 6 tests):

- First coverage for `request_affinity_key` (route-gated, length-bounded) and `gateway_turn_input` (Claude Code + Anthropic Messages only).
- Header-only session-id routing on the Models and Count-Tokens routes, contrasted with the body fallbacks on the Responses/Chat routes.
- Gateway route name round-trips.

**Codec parity** (`crates/core/tests/unit/codec/parity_tests.rs`, new, 14 tests):

Each test builds the same logical scenario in all three provider schemas (OpenAI Chat Completions, Anthropic Messages, OpenAI Responses) and asserts the normalized output agrees: model name, finish reasons, tool calls (full-struct equality), usage incl. cache-read tokens, provider-reported and catalog-estimated cost, hint hardening (`normalize_request_with_hint`), and request normalization. Schema-inherent divergences are asserted explicitly as part of the parity contract, e.g. Responses normalizes a tool-call turn to `Complete` (no tool-use terminal status), cache-write tokens are Anthropic-only, reasoning tokens are Responses-only.

**Exporter parity** (`crates/core/tests/unit/observability/exporter_parity_tests.rs`, new, 9 tests):

A shared harness feeds one event stream to the ATIF exporter, `OpenTelemetrySubscriber`, and `OpenInferenceSubscriber` (in-memory span exporters) and asserts the projected facts agree: cost totals, usage, model names, tool-call projection, replay-payload preservation, and the consolidated manual-fallback path — run against both OpenAI-Chat-shaped and Anthropic-shaped payloads. Intentional projection divergences are pinned with explicit assertions so drift fails loudly, notably: OTel emits no token-count attributes, reasoning facts are ATIF-only, ATIF sums cache read+write while OpenInference splits them, and OpenInference omits raw LLM request JSON. Cost-currency policy boundaries stay covered by the existing per-exporter tests from #304 and are not duplicated here.

**Validation**

- `just test-rust` — full workspace green
- `cargo clippy --workspace --all-targets -- -D warnings` — clean
- `cargo fmt --all --check` — clean
- `uv run pre-commit run --files <changed files>` — all hooks green (SPDX, fmt, clippy, check, linkcheck)
- Rebased onto current `main` and re-ran `cargo test -p nemo-relay` and `cargo test -p nemo-relay-cli` — green, including the parity suites against the latest observability changes
- Binding matrix not run: the change is test-only; the `#[cfg(test)]` modules are not compiled into the library targets, so binding-facing behavior is unchanged

**Breaking changes**

None.

#### Where should the reviewer start?

`crates/core/tests/unit/observability/exporter_parity_tests.rs` — the `export_through_all_exporters` harness and the explicitly pinned divergences. These assertions encode current behavior. The OTel cost-only projection and the ATIF cache-sum vs OpenInference cache-split asymmetry match the token/cost field-semantics contract documented in #330; the remaining pinned divergences (reasoning projection being ATIF-only, OTel not flattening LLM tool calls, OpenInference omitting raw LLM request JSON) encode current behavior without a recorded decision — if any is ruled a defect, the fix should land together with flipping the pinning assertion.

Then `crates/core/tests/unit/codec/parity_tests.rs` for the cross-schema normalization contract, and the adapter/alignment additions in `crates/cli/tests/coverage/`.

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

- Relates to: none (test backfill for the extraction-strategy refactors #291, #300, #301, #304, #318)



## Summary by CodeRabbit

* **Tests**
  * Added coverage for Hermes and Claude tool adapter/extractor precedence, partial payload handling, and hint/session/model/request-id resolution (including null/empty-string and fallback-chain behaviors).
  * Expanded gateway alignment tests for affinity-key/session-id gating and route/provider-specific prompt building.
  * Introduced core codec and observability cross-provider/exporter parity tests validating normalized fields, tool-call mapping, and consistent usage/cost/model/replay projections (with expected divergence checks).

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Will Killian (https://github.com/willkill07)
  - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv)
  - Maryam Najafian (https://github.com/mnajafian-nv)

URL: #355
willkill07 pushed a commit that referenced this pull request Jul 3, 2026
#### Overview

Document and stabilize NeMo Relay's LLM token and cost field semantics. This freezes the current behavior as a documented contract and locks it with characterization tests. There is no runtime behavior change. The branch is merged with current `main`, so the docs and tests reflect #304 (prefer normalized exporter fallbacks) and #305 (shared `nemo_relay_types` DTO crate).

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

#### Details

Adds a canonical **Token and Cost Field Semantics** section to `docs/integrate-into-frameworks/provider-response-codecs.mdx`:

- `Usage` and `CostEstimate` field reference (names, units, optionality; optional fields can also be codec-computed or pricing-synthesized).
- Per-provider token normalization table (OpenAI Chat / OpenAI Responses / Anthropic → `Usage`).
- **Granularity**: per-call values; an exported call yields a `user` start step (no metrics) and an `agent` end step that carries metrics; `final_metrics` is the only aggregate (a per-trajectory sum of present step metrics, excluding embedded subagent trajectories).
- Exporter field-mapping table across ATOF / ATIF / OpenInference / OpenTelemetry: OpenTelemetry is cost-only and currency-aware; ATIF and OpenInference are USD-only; ATIF sources metrics from the codec-normalized usage with raw-payload fallback.
- A **Stability** subsection (stable as of ATOF `0.1` / `ATIF-v1.7` / pricing catalog `version: 1`). The additive-compatibility promise is scoped to the serialized JSON/ATOF shapes; the Rust `Usage`/`CostEstimate` structs and `CostSource` enum are exhaustive, so adding a field/variant is source-breaking for Rust consumers.

Short field pointers + back-links were added to the OpenTelemetry, OpenInference, and ATIF exporter pages. Cost policy is stated once on the canonical page, per the runtime-contract docs convention.

Two characterization tests lock the freeze:

- The OpenTelemetry LLM span emits cost only — driven end-to-end through `OtelEventProcessor`, asserting the finished span carries `nemo_relay.llm.cost.{total,currency}` and no token-count attribute in any namespace (`token`-containing or `gen_ai.*` keys fail).
- `OpenAIChatCodec` drops `completion_tokens_details`, and `Usage` ignores unmodeled provider subfields (forward-compat: no serde catch-all).

Existing tests already cover the remaining projections, per-provider mapping, reasoning-tokens-in-`api_specific`, and the USD-only/currency-aware cost behavior.

**Testing:** targeted `cargo test` on the merged tree (the new/changed tests pass; perturbing the OTel exporter with a `nemo_relay.prompt_tokens` attribute makes the span test fail as intended, then reverted), `just docs-linkcheck` (0 errors), and `pre-commit` (SPDX, markdown linkcheck, cargo fmt/clippy/check) all pass.

#### Where should the reviewer start?

`docs/integrate-into-frameworks/provider-response-codecs.mdx` — the **Token and Cost Field Semantics** section (the exporter field-mapping table and the **Stability** subsection are the core contract). Then `crates/core/tests/unit/observability/otel_tests.rs::llm_end_emits_cost_only_no_token_or_gen_ai_attributes`.

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

- N/A

## Summary by CodeRabbit

* **Bug Fixes**
  * Response decoding now preserves supported `usage` token totals while omitting provider-only completion token detail fields.
  * `Usage` serialization now ignores unknown/unmodeled provider subfields.
  * Observability “LLM end” spans now emit only total cost attributes (no token-related or `gen_ai*` attributes).
* **Tests**
  * Added unit tests covering decoding behavior, handling of unmodeled `usage` fields, and cost-only observability attributes.
* **Documentation**
  * Added/expanded stable “Token and Cost Field Semantics” mappings, including updates for ATIF, OpenInference, and OpenTelemetry expected-output fields.

Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
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.

3 participants