Skip to content

fix: preserve successful terminal stream status - #741

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:release/0.7from
AjayThorve:fix/terminal-stream-drop-status
Aug 8, 2026
Merged

fix: preserve successful terminal stream status#741
rapids-bot[bot] merged 2 commits into
NVIDIA:release/0.7from
AjayThorve:fix/terminal-stream-drop-status

Conversation

@AjayThorve

@AjayThorve AjayThorve commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Overview

Prevent a successfully completed streamed LLM call from being exported as an OpenTelemetry error when a client stops polling after the provider's terminal response but before transport EOF.

  • 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

  • Distinguish clean stream exhaustion, real stream failure, and client-drop termination internally.
  • Override only the synthetic client-drop error when the finalized response contains both a known terminal provider outcome and authoritative final token usage.
  • Preserve ERROR for partial drops, unknown terminal states, collector/codec failures, and upstream transport failures.
  • Add an integration regression using the real OpenAI Responses streaming/final response codecs, while retaining coverage that partial drops and unknown outcomes stay errors.

Validation completed:

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • just test-rust
  • just ci=true test-rust (3,859 passed; CI-profile coverage report generated)
  • just test-python (639 passed)
  • just test-node (353 passed with Node 24.15.0)
  • uv run pre-commit run --all-files
  • Focused stream regression tests (3 integration and 1 unit test passed)
  • Tokenomics OTel capture with Relay built from this branch and Codex 0.146.1: 10/10 GenAI LLM spans were STATUS_CODE_OK with finish_reason=stop; no error-status spans appeared in any of the 30 Full, GenAI, and OpenInference exports.
  • Hosted PR matrix: 69 successful checks, 10 expected skips, 0 failures, and 0 pending checks.

just test-go has three pre-existing failures in stale observability-config tests that still use the removed standalone OpenInference v3 field. The same three tests fail on the untouched release/0.7 head (5f3c210e); all Go stream tests pass.

Breaking changes: none.

Where should the reviewer start?

Start with crates/core/src/stream.rs, specifically the termination classification passed into emit_end_event, then review dropped_stream_after_terminal_response_emits_success in crates/core/tests/integration/stream_tests.rs.

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

  • Relates to: none

Signed-off-by: Ajay Thorve <athorve@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Stream termination now distinguishes completion, failure, and drop states. Authoritative completion metadata can classify dropped streams as successful. Tests cover successful terminal responses and unknown finish reasons.

Changes

Stream termination handling

Layer / File(s) Summary
Explicit stream termination states
crates/core/src/stream.rs
The stream uses Complete, Failed, and Dropped termination variants. Normal exhaustion and errors use dedicated completion paths.
Completion evidence and validation
crates/core/src/stream.rs, crates/core/tests/integration/stream_tests.rs, crates/core/tests/unit/stream_tests.rs
Dropped streams report OK when final usage is authoritative and FinishReason is known and not Unknown. Tests cover terminal response events and unknown failure reasons.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ResponsesStream
  participant StreamTermination
  participant EndEvent
  ResponsesStream->>StreamTermination: receive response.completed
  ResponsesStream->>StreamTermination: drop stream wrapper
  StreamTermination->>StreamTermination: validate final usage and FinishReason
  StreamTermination->>EndEvent: emit OK status without description
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title follows Conventional Commits format, uses an allowed type, states the fix clearly, and is under 72 characters.
Description check ✅ Passed The description includes all required sections, explains the change and validation, identifies review starting points, and uses the required related-issues keyword.
✨ 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:M PR is medium Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code labels Aug 7, 2026
@AjayThorve
AjayThorve marked this pull request as ready for review August 7, 2026 23:45
@AjayThorve
AjayThorve requested a review from a team as a code owner August 7, 2026 23:45
@AjayThorve AjayThorve self-assigned this Aug 7, 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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/core/src/stream.rs`:
- Around line 533-540: Update has_authoritative_successful_completion to require
FinishReason::is_complete() instead of merely excluding FinishReason::Unknown,
while preserving the authoritative usage requirement. In
crates/core/tests/unit/stream_tests.rs:38-45, add stream tests covering Length,
ToolUse, and ContentFilter to verify dropped non-complete responses are not
reported with otel.status_code = OK.
- Around line 299-308: Complete and record all required validation checks for
the crates/core changes: cargo fmt --all, just test-rust, just ci=true
test-rust, cargo clippy --workspace --all-targets -- -D warnings, just
test-python, just test-go, just test-node, and uv run pre-commit run
--all-files. Wait for the in-progress CI matrix and mark it complete only after
every check passes.
🪄 Autofix

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: e5a9ee67-404b-4bef-873b-581da73a763b

📥 Commits

Reviewing files that changed from the base of the PR and between 5f3c210 and a89964b.

📒 Files selected for processing (3)
  • crates/core/src/stream.rs
  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_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 (20)
**/*.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

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

**/*.rs: Format Rust code with rustfmt defaults using cargo fmt.
Run cargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rust snake_case naming conventions.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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

Use Result<T> with FlowError in core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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.

**/*: Use release tags in raw Rust-compatible SemVer without a leading v; tags such as v0.1.0 are prohibited.
Use branch prefixes feat/, fix/, docs/, test/, or refactor/ according to the change purpose.
Every commit in a pull request must include a DCO Signed-off-by: sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latest main are complete.
Use commit messages in the form type: short description, with a valid type and a first line under 72 characters.

Run the prescribed plugin validation commands, including fixture building, focused Rust and Python package tests, integration tests, documentation checks, and the broader validate-change matrix for broad runtime or public API changes.

**/*: Keep observability changes scoped, surface assumptions, and define focused validation before editing.
Run affected Rust tests and just test-rust for event-field changes; run just test-python, just test-go, and just test-node when binding-native configuration or lifecycle changes; update docs and examples in the same branch.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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.

For shared-semantics or broad runtime changes in the core or adaptive crates, run just ci=true test-rust.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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.

**/*.{rs,py,go,js,ts}: Keep FFI and Python, Go, and Node.js binding configuration objects and subscriber/exporter methods aligned with the core observability configuration and lifecycle semantics.
Preserve complete sanitized LLM request input and annotations when enable_full_payloads is enabled, while retaining credential removal and sanitizers.
Use each exporter's documented flush and deregister order before shutdown in observability examples and implementations.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}

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

Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Use snake_case naming in Rust and Python.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.

crates/**/*.rs: Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
plugins list, plugins inspect, and plugins validate must report lifecycle and compatibility status without leaking secret configuration.
Top-level doctor must report resolved dynamic-plugin and host-configuration status.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,mjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
Use SONAR_IGNORE_START / SONAR_IGNORE_END only for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/{test,tests}/**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

When adding functionality, include tests in the appropriate test files for each affected language binding.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{toml,md,rs,py}

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

Treat plugin Relay compatibility as normal SemVer; use >=0.5,<1.0 in examples unless a plugin intentionally declares a narrower range.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)

For changes in the Rust core, adaptive, dynamic plugin, worker, worker-proto, or types crates, run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings as the default validation sequence.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*

📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)

crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*: For changes affecting crates/core, crates/adaptive, or shared Rust runtime semantics, expand validation to the full binding matrix with validate-change.
Use narrower crate-specific tests only as a local debug loop, not as the final validation for a Rust change.
If a public API, event shape, middleware behavior, plugin semantics, or crates/core/crates/adaptive behavior changes, also run validate-change.
If the change is isolated to one binding wrapper while Rust semantics remain unchanged, prefer that binding's build/test skill instead.

Files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
crates/**/src/**/*.rs

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

Do not add tests under src; Rust tests belong in crate tests/ trees.

Files:

  • crates/core/src/stream.rs
🧠 Learnings (2)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.

Applied to files:

  • crates/core/tests/integration/stream_tests.rs
  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.

Applied to files:

  • crates/core/src/stream.rs
🔇 Additional comments (2)
crates/core/src/stream.rs (1)

51-53: LGTM!

Also applies to: 91-235, 245-245, 480-492

crates/core/tests/integration/stream_tests.rs (1)

21-23: LGTM!

Also applies to: 476-540

Comment thread crates/core/src/stream.rs
Comment thread crates/core/src/stream.rs Outdated
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Signed-off-by: Ajay Thorve <athorve@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
crates/core/src/stream.rs (2)

218-225: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep codec failures on the error path.

finish_cleanly starts finalization with OK metadata. At Line 292, decode_response(...).ok()? converts a codec error into None. The later status logic does not change metadata for StreamTermination::Complete, so a cleanly exhausted stream with a response-codec failure is published as OK.

Propagate the codec error into metadata_with_otel_error before end_llm_handle emits the event. Add a regression test for clean EOF with a codec failure.

🤖 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/stream.rs` around lines 218 - 225, Update finish_cleanly and
the decode_response flow so codec failures are retained and passed to
metadata_with_otel_error before end_llm_handle emits the completion event,
instead of being discarded by .ok()? and reported as OK. Preserve normal
clean-EOF behavior for successful decoding, and add a regression test covering
clean EOF with a response-codec failure.

480-492: 📐 Maintainability & Code Quality | 🟠 Major

Complete the required core validation before merge.

The PR objective reports Node and pre-commit validation as incomplete and three pre-existing Go test failures. Because crates/core changed, run and record cargo fmt --all, just test-rust, just ci=true test-rust, cargo clippy --workspace --all-targets -- -D warnings, validate-change, just test-python, just test-go, just test-node, and uv run pre-commit run --all-files. Do not mark the validation matrix complete while a required target is pending or failing.

🤖 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/stream.rs` around lines 480 - 492, Complete and record all
required validation commands for the crates/core change: cargo fmt --all, just
test-rust, just ci=true test-rust, cargo clippy --workspace --all-targets -- -D
warnings, validate-change, just test-python, just test-go, just test-node, and
uv run pre-commit run --all-files. Resolve or explicitly report any failures,
including the Go failures, and do not mark the validation matrix complete while
any required check remains pending or failing.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/core/src/stream.rs`:
- Around line 218-225: Update finish_cleanly and the decode_response flow so
codec failures are retained and passed to metadata_with_otel_error before
end_llm_handle emits the completion event, instead of being discarded by .ok()?
and reported as OK. Preserve normal clean-EOF behavior for successful decoding,
and add a regression test covering clean EOF with a response-codec failure.
- Around line 480-492: Complete and record all required validation commands for
the crates/core change: cargo fmt --all, just test-rust, just ci=true test-rust,
cargo clippy --workspace --all-targets -- -D warnings, validate-change, just
test-python, just test-go, just test-node, and uv run pre-commit run
--all-files. Resolve or explicitly report any failures, including the Go
failures, and do not mark the validation matrix complete while any required
check remains pending or failing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: e69cfc84-489c-42c4-ac55-43f9d0463202

📥 Commits

Reviewing files that changed from the base of the PR and between a89964b and c736871.

📒 Files selected for processing (2)
  • crates/core/src/stream.rs
  • crates/core/tests/unit/stream_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (20)
**/*.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

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

**/*.rs: Format Rust code with rustfmt defaults using cargo fmt.
Run cargo clippy -- -D warnings; all Rust warnings must be treated as errors.
Use Rust snake_case naming conventions.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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/stream_tests.rs
  • crates/core/src/stream.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

Use Result<T> with FlowError in core runtime paths, keeping wrapper-layer errors explicit and binding-appropriate.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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.

**/*: Use release tags in raw Rust-compatible SemVer without a leading v; tags such as v0.1.0 are prohibited.
Use branch prefixes feat/, fix/, docs/, test/, or refactor/ according to the change purpose.
Every commit in a pull request must include a DCO Signed-off-by: sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latest main are complete.
Use commit messages in the form type: short description, with a valid type and a first line under 72 characters.

Run the prescribed plugin validation commands, including fixture building, focused Rust and Python package tests, integration tests, documentation checks, and the broader validate-change matrix for broad runtime or public API changes.

**/*: Keep observability changes scoped, surface assumptions, and define focused validation before editing.
Run affected Rust tests and just test-rust for event-field changes; run just test-python, just test-go, and just test-node when binding-native configuration or lifecycle changes; update docs and examples in the same branch.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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.

For shared-semantics or broad runtime changes in the core or adaptive crates, run just ci=true test-rust.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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.

**/*.{rs,py,go,js,ts}: Keep FFI and Python, Go, and Node.js binding configuration objects and subscriber/exporter methods aligned with the core observability configuration and lifecycle semantics.
Preserve complete sanitized LLM request input and annotations when enable_full_payloads is enabled, while retaining credential removal and sanitizers.
Use each exporter's documented flush and deregister order before shutdown in observability examples and implementations.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}

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

Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Use snake_case naming in Rust and Python.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where existing code expects JSON payloads.
Treat Rust as the source of truth for runtime behavior; binding APIs should mirror Rust semantics unless a language-specific wrapper intentionally improves ergonomics.

crates/**/*.rs: Runtime helpers must cover marks, scopes, continuations, and isolated scope stacks.
plugins list, plugins inspect, and plugins validate must report lifecycle and compatibility status without leaking secret configuration.
Top-level doctor must report resolved dynamic-plugin and host-configuration status.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,mjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
Use SONAR_IGNORE_START / SONAR_IGNORE_END only for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/{test,tests}/**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

When adding functionality, include tests in the appropriate test files for each affected language binding.

Files:

  • crates/core/tests/unit/stream_tests.rs
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
**/*.{toml,md,rs,py}

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

Treat plugin Relay compatibility as normal SemVer; use >=0.5,<1.0 in examples unless a plugin intentionally declares a narrower range.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)

For changes in the Rust core, adaptive, dynamic plugin, worker, worker-proto, or types crates, run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings as the default validation sequence.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*

📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)

crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*: For changes affecting crates/core, crates/adaptive, or shared Rust runtime semantics, expand validation to the full binding matrix with validate-change.
Use narrower crate-specific tests only as a local debug loop, not as the final validation for a Rust change.
If a public API, event shape, middleware behavior, plugin semantics, or crates/core/crates/adaptive behavior changes, also run validate-change.
If the change is isolated to one binding wrapper while Rust semantics remain unchanged, prefer that binding's build/test skill instead.

Files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.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/stream_tests.rs
  • crates/core/src/stream.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/stream_tests.rs
crates/**/src/**/*.rs

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

Do not add tests under src; Rust tests belong in crate tests/ trees.

Files:

  • crates/core/src/stream.rs
🧠 Learnings (2)
📚 Learning: 2026-08-03T19:55:03.931Z
Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.

Applied to files:

  • crates/core/tests/unit/stream_tests.rs
  • crates/core/src/stream.rs
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.

Applied to files:

  • crates/core/src/stream.rs
🔇 Additional comments (1)
crates/core/src/stream.rs (1)

51-53: LGTM!

Also applies to: 91-103, 214-217, 228-235, 245-245

@willkill07

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit a6a8572 into NVIDIA:release/0.7 Aug 8, 2026
82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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