Skip to content

fix(observability)!: attribute ATIF steps to effective response model#399

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
bbednarski9:fix/atif-effective-model-attribution
Jul 14, 2026
Merged

fix(observability)!: attribute ATIF steps to effective response model#399
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
bbednarski9:fix/atif-effective-model-attribution

Conversation

@bbednarski9

@bbednarski9 bbednarski9 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Warning

BREAKING CHANGE: [Behavioral] ATIF LLM steps now prefer the model reported by the normalized provider response over the requested or profile model. Consumers that group, compare, or price trajectories by step.model_name may observe a different model for routed or cross-protocol calls. When no effective response model is available, the existing attribution fallback remains unchanged.

Overview

Correct ATIF model attribution for LLM calls whose effective provider model differs from the model requested at the beginning of the Relay lifecycle.

This generic exporter fix is intentionally independent of the Switchyard plugin. Any router, proxy, or provider translation layer that records the effective model in AnnotatedLlmResponse receives the same ATIF behavior.

  • 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

ATIF previously preferred the model recorded on the LLM start event. For routed requests, that value can be a client alias, profile model, or originally requested model rather than the backend that actually produced the response. The normalized end response already contains the effective model, but ATIF did not use it for step.model_name.

This PR:

  • Adds one shared effective-response model selector.
  • Uses the normalized response model for paired start/end LLM spans.
  • Uses the same selection for standalone LLM event conversion.
  • Falls back to the existing event-model extraction when the response does not identify a model.
  • Adds a regression where the request is attributed to qwen3:4b but the normalized response is produced by qwen3.6:35b.

No ATOF event, provider response, routing decision, cost calculation, or plugin behavior changes. Only exported ATIF step.model_name attribution changes.

Validation:

  • cargo fmt --all -- --check
  • cargo test -p nemo-relay test_exporter_prefers_effective_response_model_over_requested_profile_model
  • cargo test -p nemo-relay observability::atif::tests — 88 passed.
  • cargo clippy -p nemo-relay --all-targets -- -D warnings
  • git diff --check

Where should the reviewer start?

  1. crates/core/src/observability/atif.rs — model selection and fallback behavior.
  2. crates/core/tests/unit/atif_tests.rs — routed-model regression fixture.

The central design decision is that ATIF describes the model that produced the response, while the requested model remains available in the original ATOF lifecycle input and optimization metadata.

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

  • Relates to NVIDIA/NeMo-Relay #369, where this correction was originally discovered and has now been removed from that diff.
  • Relates to NVIDIA/NeMo-Relay #389, which records effective model and optimization evidence on the normalized LLM response.

Summary by CodeRabbit

  • Bug Fixes
    • Agent step records now use the effective model from normalized/effective LLM response metadata when available, improving model accuracy when start/end event details differ.
    • ATIF span candidate deduplication and emitted step model identification were improved to prefer the normalized response’s model over requested/profile-only values.
  • Tests
    • Added unit tests covering mismatched requested vs. effective models and verifying model selection behavior for both span candidates and exported steps.

Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 38eea4d4-71c6-411e-8cbc-b96883043471

📥 Commits

Reviewing files that changed from the base of the PR and between e7d73f3 and a6c42e4.

📒 Files selected for processing (1)
  • crates/core/src/observability/atif.rs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (13)
**/*.rs

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

**/*.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

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.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.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

  • 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/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

Files:

  • crates/core/src/observability/atif.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

  • crates/core/src/observability/atif.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: 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, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.

Files:

  • crates/core/src/observability/atif.rs
**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

  • crates/core/src/observability/atif.rs
**/*.{rs,go,js,ts}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding // comment form.

Files:

  • crates/core/src/observability/atif.rs
{crates/**/src/**/*.rs,python/**/*.py}

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

Do not add tests under src; Rust tests belong in crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

  • crates/core/src/observability/atif.rs
crates/core/src/observability/{atif,otel,openinference}.rs

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

When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.

Files:

  • crates/core/src/observability/atif.rs
**/*

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

**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, use maintain-dynamic-plugins and include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

  • 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 validation matrix across Rust, Python, Go, and Node.js.

Files:

  • crates/core/src/observability/atif.rs
**/*.{rs,py,go,js,ts}

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

If a language surface changed, always run that language's test target even when Rust core did not change.

Files:

  • crates/core/src/observability/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/src/observability/atif.rs
🔇 Additional comments (2)
crates/core/src/observability/atif.rs (2)

1515-1515: LGTM!

Also applies to: 2375-2375, 2706-2719


2706-2719: 📐 Maintainability & Code Quality

Confirm the required Rust/core validation before merge.

Verify cargo fmt --all, just test-rust, cargo clippy --workspace --all-targets -- -D warnings, validate-change, the full Rust/Python/Go/Node matrix, and uv run pre-commit run --all-files were run against the final tree.

As per coding guidelines, Rust and crates/core changes require these formatting, lint, test, pre-commit, and broader validation steps.

Source: Coding guidelines


Walkthrough

ATIF model selection now prefers the normalized LLM response model for emitted steps and span candidates, while retaining event-based fallbacks. Unit tests cover effective-model preference and Start-event fallback.

Changes

ATIF model selection

Layer / File(s) Summary
Resolve and validate effective response models
crates/core/src/observability/atif.rs, crates/core/tests/unit/atif_tests.rs
The exporter centralizes response-model fallback logic, applies it to step and span handling, and verifies effective response selection and Start-event fallback behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the breaking observability fix.
Description check ✅ Passed The description matches the template sections and includes overview, details, reviewer start, validation, and related items.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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 size:S PR is small Bug issue describes bug; PR fixes bug breaking PR introduces a breaking change lang:rust PR changes/introduces Rust code labels Jul 9, 2026
@willkill07 willkill07 added this to the 0.6 milestone Jul 10, 2026
rapids-bot Bot pushed a commit that referenced this pull request Jul 13, 2026
> [!WARNING]
> **BREAKING CHANGE:** **[Rust source compatibility]** This PR adds `FlowError::Upstream` and new fields to the public `AtofEndpointConfig`, `AtofEndpointSectionConfig`, and `EmitMarkEventParams` types. Downstream exhaustive `FlowError` matches must handle the new variant, and direct struct literals must initialize the new fields or migrate to the provided constructor/builder APIs. Builder-based mark emission and default Relay configurations remain compatible.

#### Overview

This adds Relay's first-party integration with the [Switchyard](https://github.com/NVIDIA-NeMo/Switchyard) Decision API. The plugin routes buffered and streaming LLM requests through an external Switchyard service, executes the selected Relay-owned backend, and records causal model-routing optimization evidence.

Switchyard owns provider-protocol translation through its `switchyard-translation` Rust library. Relay consumes that library only when the native Switchyard plugin is enabled; default Relay builds do not include either the plugin or the translation dependency.

- [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

##### Plugin and execution behavior

- Add the standalone `nemo-relay-switchyard` first-party plugin.
- Normalize stable Relay identity and build `switchyard.routing_request.v1` requests.
- Validate selected routes and optional counterfactual baselines against exact Relay-owned backend bindings.
- Support enforce and observe-only modes, six request-materialization modes, bounded retry-aware dispatch, protocol-specific trusted fallbacks, and no retry after the first upstream stream item.
- Record one model-routing optimization contribution only after the selected route commits. Failed decisions, failed provider attempts, and fallback dispatches do not claim routing savings.
- Preserve Switchyard routing metadata, including snapshot freshness, decision source, event count, and turn depth.
- Continue using Relay's existing ATOF exporter and lifecycle; the plugin does not install local routers, feature accumulators, or a duplicate event subscriber.

##### Translation ownership and optional dependency

Request, buffered-response, and SSE translation across OpenAI Chat Completions, OpenAI Responses, and Anthropic Messages use Switchyard's public `switchyard-translation` library instead of a Relay-local translation engine.

The dependency is pinned to the immutable topic-branch commit [`8f9db9a6a47f848cdff1d262276ba25a8ae9cbc8`](NVIDIA-NeMo/Switchyard@8f9db9a), which includes the Decision API series, StageRouter baseline/freshness behavior, and [Switchyard #46](NVIDIA-NeMo/Switchyard#46).

The Relay CLI exposes this through `--features switchyard`:

- Without that feature, `nemo-relay-cli` contains neither `nemo-relay-switchyard` nor `switchyard-translation`.
- With that feature, the plugin and its pinned Switchyard translation library are compiled and registered.
- Relay continues to own target bindings, credentials, dispatch, retries, fallbacks, lifecycle, and observability.
- Relay retains narrow fail-open policy checks for provider extensions that cannot be translated without data loss.

##### Runtime requirements

The current experimental deployment requires a separately running Switchyard service from the pinned topic commit. The plugin does not start or supervise that service.

Switchyard availability is not a Relay startup requirement. It is a request-time dependency: connection failures, timeouts, malformed decisions, selected-route drift, unsupported extensions, and missing required identity fail open to the configured same-protocol default. ATOF-backed profiles additionally require Relay's ATOF HTTP exporter to target Switchyard's `/v1/atof/events` endpoint with preserved field names and environment-referenced authentication.

The public examples document this service boundary, verify the exact Switchyard revision before startup, and intentionally remain manual compatibility workflows rather than production orchestration.

##### Optimization accounting and observability

This PR builds on the plugin-neutral accounting contract merged in [NeMo Relay #389](#389). Applied routing contributions carry the selected and baseline model transition, decision identity, backend and tier, attempt, rollout mode, reason, and router metadata. Observe-only decisions remain visible as non-applied evidence.

Relay core combines routing and other plugin contributions at LLM close while retaining explicit token evidence for downstream repricing. Existing requested, decision, retry, fallback, and error routing marks remain available alongside the canonical optimization contribution.

The generic ATIF change that attributes `step.model_name` to the effective response model is intentionally separated into [NeMo Relay #399](#399). This PR still ensures translated provider responses carry the actual selected model; #399 independently controls how ATIF presents it.

##### Testing summary

- `cargo fmt --all -- --check`
- `cargo clippy -p nemo-relay-switchyard --all-targets -- -D warnings`
- `cargo clippy -p nemo-relay-cli --features switchyard --all-targets -- -D warnings`
- `cargo check -p nemo-relay-cli` — proves the default CLI builds without Switchyard.
- `cargo check -p nemo-relay-cli --features switchyard` — proves the opt-in dependency path.
- `cargo test -p nemo-relay-switchyard` — 31 passed.
- `cargo test -p nemo-relay-cli gateway::tests` — 33 passed without the feature.
- `cargo test -p nemo-relay-cli --features switchyard gateway::tests` — 33 passed with the feature.
- `cargo test -p nemo-relay-cli --features switchyard --test switchyard_process_e2e` — buffered, streaming, retry, and fail-open process boundary passed after the split.
- `examples/switchyard/run-real-e2e.sh` against the exact pinned Switchyard service — passed with the expected StageRouter route sequence `provider/weak`, `provider/strong`, `provider/strong`.
- The pinned Switchyard translation crate separately passes 61 request, response, streaming, extension, and round-trip tests under Rust 1.93.

##### Previous feedback addressed

- Split the implementation at repository ownership boundaries.
- Replaced Relay's custom translation engine with the Switchyard-owned reusable library.
- Kept the dependency compile-time optional for users who do not enable the Relay-native Switchyard plugin.
- Removed in-process Random, LLM, and Cascade routers, Relay-side feature accumulation, duplicate Switchyard subscriptions, and the global fallback target.
- Replaced literal credentials with environment-referenced headers and activation-time validation.
- Hardened internal dispatch headers and structured upstream-failure metadata.
- Standardized routing terminology on StageRouter.
- Standardized model-routing evidence on Relay's shared optimization contract instead of adding a second post-call savings mark.
- Moved generic ATIF effective-model attribution into focused PR #399.
- Moved Fern documentation into focused experimental docs PR #400 so this functional PR remains code-focused.
- Kept public examples free of internal-only providers, credentials, Visor source references, and private infrastructure assumptions.

#### Where should the reviewer start?

1. `crates/switchyard/src/component.rs` — Decision API lifecycle, exact target and baseline validation, retries, fallback, and terminal route commitment.
2. `crates/switchyard/src/translation.rs` and `stream_translation.rs` — thin policy adapters over the Switchyard-owned translation library.
3. `crates/cli/Cargo.toml` and the `cfg(feature = "switchyard")` registration paths — compile-time optionality.
4. `crates/cli/src/gateway.rs` — reserved dispatch headers and structured retry-aware upstream failures.
5. `crates/cli/tests/switchyard_process_e2e.rs` — process-boundary buffered, streaming, and fail-open coverage.
6. `examples/switchyard/README.md` and `run-real-e2e.sh` — pinned experimental service workflow.

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

- Relates to [NVIDIA/NeMo-Relay #389](#389).
- Relates to [NVIDIA/NeMo-Relay #399](#399).
- Relates to [NVIDIA/NeMo-Relay #400](#400).
- Relates to [NVIDIA-NeMo/Switchyard #46](NVIDIA-NeMo/Switchyard#46).
- Paired with the Switchyard Decision API topic series:
  - [NVIDIA-NeMo/Switchyard #8](NVIDIA-NeMo/Switchyard#8)
  - [NVIDIA-NeMo/Switchyard #9](NVIDIA-NeMo/Switchyard#9)
  - [NVIDIA-NeMo/Switchyard #11](NVIDIA-NeMo/Switchyard#11)
  - [NVIDIA-NeMo/Switchyard #35](NVIDIA-NeMo/Switchyard#35)

Authors:
  - Bryan Bednarski (https://github.com/bbednarski9)

Approvers:
  - Will Killian (https://github.com/willkill07)
  - Maryam Najafian (https://github.com/mnajafian-nv)
  - https://github.com/Salonijain27

URL: #369
@bbednarski9
bbednarski9 marked this pull request as ready for review July 14, 2026 13:48
@bbednarski9
bbednarski9 requested a review from a team as a code owner July 14, 2026 13:48

@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 1515-1516: Update the paired-span model attribution around
effective_response_model_name so it distinguishes the response-only normalized
lookup from the fallback to end’s event model, preserving the existing order
before consulting model_name_for_llm_event(start). Add a regression case
covering different start and end models with no normalized response, asserting
the intended paired-span fallback attribution.
🪄 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: 726c31ea-b2df-4b93-bfcc-00aedcf806f6

📥 Commits

Reviewing files that changed from the base of the PR and between 8687117 and 79905e8.

📒 Files selected for processing (2)
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (14)
**/*.rs

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

**/*.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

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.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.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

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

Files:

  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: 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, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.

Files:

  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

  • crates/core/tests/unit/atif_tests.rs
  • crates/core/src/observability/atif.rs
**/*.{rs,go,js,ts}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding // comment form.

Files:

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

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

**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, use maintain-dynamic-plugins and include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

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 validation matrix across Rust, Python, Go, and Node.js.

Files:

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

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

If a language surface changed, always run that language's test target even when Rust core did not change.

Files:

  • crates/core/tests/unit/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/**/src/**/*.rs,python/**/*.py}

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

Do not add tests under src; Rust tests belong in crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

  • crates/core/src/observability/atif.rs
crates/core/src/observability/{atif,otel,openinference}.rs

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

When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.

Files:

  • crates/core/src/observability/atif.rs
🔇 Additional comments (3)
crates/core/src/observability/atif.rs (2)

2376-2376: 📐 Maintainability & Code Quality

Provide the required core-Rust validation results.

The PR summary mentions targeted tests, formatting, and Clippy, but this crates/core behavior change also requires evidence for just test-rust, cargo fmt --all, strict Clippy, cargo deny check, validate-change, the Rust/Python/Go/Node matrix, and uv run pre-commit run --all-files.

As per coding guidelines, “If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation” and “If crates/core or crates/adaptive changed, run the full validation matrix across Rust, Python, Go, and Node.js.”

Source: Coding guidelines


2701-2706: LGTM!

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

882-933: LGTM!

Comment thread crates/core/src/observability/atif.rs Outdated
@willkill07

Copy link
Copy Markdown
Member

/ok to test 79905e8

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

May want to address given Coderabbit feedback. Non-blocking.

LGTM

@github-actions

Copy link
Copy Markdown

Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>

@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 1515-1523: Extract the shared response-model fallback into a
helper such as effective_model_for_pair, preserving the order
normalized_response_model_name → explicit model_name() →
model_name_for_llm_event(start) → model_name_for_llm_event(end). Update both
LlmSpanCandidate::from_events and effective_response_model_name, including the
handle_llm_end path, to use this helper instead of maintaining separate fallback
logic.
🪄 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: eb0624e1-f419-483a-8a29-b456ac7b5d85

📥 Commits

Reviewing files that changed from the base of the PR and between 79905e8 and e7d73f3.

📒 Files selected for processing (2)
  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (14)
**/*.rs

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

**/*.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

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.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.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

  • 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/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

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: 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, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
**/*.{rs,go,js,ts}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding // comment form.

Files:

  • crates/core/src/observability/atif.rs
  • crates/core/tests/unit/atif_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}

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

Do not add tests under src; Rust tests belong in crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

  • crates/core/src/observability/atif.rs
crates/core/src/observability/{atif,otel,openinference}.rs

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

When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.

Files:

  • crates/core/src/observability/atif.rs
**/*

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

**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, use maintain-dynamic-plugins and include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

  • 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 validation matrix across Rust, Python, Go, and Node.js.

Files:

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

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

If a language surface changed, always run that language's test target even when Rust core did not change.

Files:

  • crates/core/src/observability/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/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/atif_tests.rs
🔇 Additional comments (4)
crates/core/src/observability/atif.rs (3)

1515-1523: Past fallback-order concern is now fixed.

The previously flagged issue (missing start-model fallback before model_name_for_llm_event) is resolved — the chain now falls back through start.model_name()/end.model_name() before hitting model_name_for_llm_event, and the new regression test (atif_tests.rs 882-903) covers exactly this scenario.


2383-2383: LGTM!


2708-2716: LGTM!

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

882-903: LGTM!

Also applies to: 906-956

Comment thread crates/core/src/observability/atif.rs Outdated
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
@github-actions github-actions Bot added size:M PR is medium and removed size:S PR is small labels Jul 14, 2026
@bbednarski9

Copy link
Copy Markdown
Contributor Author

/ok to test a6c42e4

@willkill07

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 7a22d53 into NVIDIA:main Jul 14, 2026
69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking PR introduces a breaking change Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code size:M PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants