Skip to content

fix: return middleware-translated buffered responses#496

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:release/0.6from
bbednarski9:bbednarski/relay-510-nemo-relay060-rc2-switchyard-v1responses-returns-chat
Jul 20, 2026
Merged

fix: return middleware-translated buffered responses#496
rapids-bot[bot] merged 3 commits into
NVIDIA:release/0.6from
bbednarski9:bbednarski/relay-510-nemo-relay060-rc2-switchyard-v1responses-returns-chat

Conversation

@bbednarski9

@bbednarski9 bbednarski9 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Overview

Fix the buffered CLI gateway so clients receive the final JSON returned by LLM middleware, including Switchyard response translations, instead of always receiving the raw upstream bytes.

  • 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

  • Serialize the final runtime JSON when middleware changes the upstream response or short-circuits without an upstream response.
  • Preserve the original upstream bytes when the semantic JSON is unchanged or the upstream body is non-JSON.
  • Preserve the actual upstream status and filtered headers.
  • Extend the Switchyard process E2E test with a non-streaming /v1/responses request, assert that the provider receives a translated Chat Completions request, and assert that the client receives an OpenAI Responses payload.
  • Keep the existing buffered, streaming, and fail-open process coverage.
  • No public API, binding, documentation, or breaking changes.

Validation:

  • cargo test -p nemo-relay-cli --features switchyard --test switchyard_process_e2e -- --nocapture
  • cargo test -p nemo-relay-cli --features switchyard -- --test-threads=1
  • cargo fmt --all
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo clippy -p nemo-relay-cli --all-targets --features switchyard -- -D warnings
  • just test-rust ran the Rust matrix; one unrelated CLI test encountered shared temporary plugin configuration from a concurrent test and passed when rerun independently.
  • uv run pre-commit run --all-files; all applicable checks passed except the baseline Rust attribution generator, which rewrites the unrelated checked-in md-5 license entry. A final all-files run with only that generator skipped passed.

Where should the reviewer start?

Start with crates/cli/src/gateway/mod.rs in run_managed_buffered, then review the process-level regression in crates/cli/tests/switchyard_process_e2e.rs.

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

  • Relates to: RELAY-510

Summary by CodeRabbit

  • Bug Fixes
    • Improved non-streaming managed gateway behavior to keep response bodies consistent with the latest processed output when discrepancies are detected, while using the original upstream body when compatible.
    • Added more robust fallback handling when buffered upstream bytes are unavailable or can’t be parsed.
  • Improvements
    • Enhanced reliability across mixed buffered, streaming, fallback, and translated /v1/responses flows to better align requests and responses.
  • Tests
    • Updated end-to-end test sequencing and strengthened assertions for translated provider request formatting.

Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
@bbednarski9
bbednarski9 requested a review from a team as a code owner July 20, 2026 20:32
@github-actions github-actions Bot added size:S PR is small Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code labels Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The buffered gateway path reconciles upstream JSON with runtime output before constructing responses. The process-boundary e2e test adds a /v1/responses translation request and updates failure timing, request ordering, and provider payload assertions.

Changes

Gateway response flow

Layer / File(s) Summary
Buffered response reconciliation
crates/cli/src/gateway/mod.rs
Buffered upstream JSON is compared with runtime output; differing or missing upstream data uses serialized runtime JSON, while matching data preserves the original bytes.
Process-boundary translation coverage
crates/cli/tests/switchyard_process_e2e.rs
The e2e sequence includes a /v1/responses request, delays the simulated failure to request four, and validates the updated provider order and translated payload shape.

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

Possibly related PRs

  • NVIDIA/NeMo-Relay#485: Updates the same buffered managed-gateway response logic around upstream JSON parsing and response-body selection.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits, is concise, and matches the buffered response translation change.
Description check ✅ Passed The description includes all required sections and relevant implementation, review, and issue details.
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.

@willkill07 willkill07 added this to the 0.6 milestone Jul 20, 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/cli/tests/switchyard_process_e2e.rs`:
- Around line 315-322: Strengthen the assertions for providers[1] in the process
boundary response test: retain the existing messages-array and absent-input
checks, then verify the translated message uses the expected user role and
contains the original “process boundary response translation” text in the
protocol’s actual message representation.
🪄 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: 47f00c97-0acb-44ca-953a-4c856e2609ed

📥 Commits

Reviewing files that changed from the base of the PR and between eebacb6 and ba9ad59.

📒 Files selected for processing (2)
  • crates/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Check / Run
🧰 Additional context used
📓 Path-based instructions (10)
**/*.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • crates/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/tests/switchyard_process_e2e.rs
🔇 Additional comments (4)
crates/cli/src/gateway/mod.rs (3)

84-84: LGTM!

Also applies to: 243-243


222-226: 🗄️ Data Integrity & Integration

Verify body-dependent headers when replacing upstream bytes.

When Lines 223-224 replace the upstream body, build_response in crates/cli/src/gateway/response.rs copies every supplied header. Confirm response_headers filters or recomputes content-length and content-encoding; otherwise clients may receive truncated or incorrectly decoded responses.


217-243: 📐 Maintainability & Code Quality

Run the required Rust checks before handoff.

The supplied context does not include results for cargo fmt --all, just test-rust, cargo clippy --workspace --all-targets -- -D warnings, or uv run pre-commit run --all-files.

As per coding guidelines, Rust changes must pass formatting, tests, clippy, and pre-commit validation.

Source: Coding guidelines

crates/cli/tests/switchyard_process_e2e.rs (1)

46-46: LGTM!

Also applies to: 239-239, 257-290

Comment thread crates/cli/tests/switchyard_process_e2e.rs
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
…ay-510-nemo-relay060-rc2-switchyard-v1responses-returns-chat

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

# Conflicts:
#	crates/cli/tests/switchyard_process_e2e.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.

Caution

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

⚠️ Outside diff range comments (2)
crates/cli/src/gateway/mod.rs (1)

217-229: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve unchanged non-JSON upstream bodies.

When upstream parsing fails, this path serializes response_json. However, build_buffered_func represents unchanged non-JSON responses as {"body_bytes": bytes.len()}, so an unchanged text, empty, or binary response is replaced by synthetic JSON instead of the original buffered bytes. Carry an explicit parse/unchanged marker so raw bytes are retained unless middleware actually changes the response.

Also applies to: 325-335

🤖 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/cli/src/gateway/mod.rs` around lines 217 - 229, Update the
response-body handling in build_buffered_func, including the corresponding path
around the additional reported range, to track whether upstream JSON parsing
succeeded and whether middleware changed the response. Preserve and return the
original buffered bytes for unchanged, non-JSON, empty, or binary bodies; only
serialize response_json when middleware actually modifies the upstream response.
crates/cli/tests/switchyard_process_e2e.rs (1)

286-288: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the /v1/responses payload content, not just object.

object == "response" still passes if the translated body drops or alters the returned content. Add an assertion on the actual response fields so this test covers response translation end to end.

🤖 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/cli/tests/switchyard_process_e2e.rs` around lines 286 - 288, Expand
the assertions in the `/v1/responses` test after deserializing `translated` to
validate the returned content fields, not only `translated["object"]`. Reuse the
expected response values established by the test and assert the translated
payload preserves them end to end.

Source: Path instructions

🤖 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/cli/src/gateway/mod.rs`:
- Around line 217-229: Update the response-body handling in build_buffered_func,
including the corresponding path around the additional reported range, to track
whether upstream JSON parsing succeeded and whether middleware changed the
response. Preserve and return the original buffered bytes for unchanged,
non-JSON, empty, or binary bodies; only serialize response_json when middleware
actually modifies the upstream response.

In `@crates/cli/tests/switchyard_process_e2e.rs`:
- Around line 286-288: Expand the assertions in the `/v1/responses` test after
deserializing `translated` to validate the returned content fields, not only
`translated["object"]`. Reuse the expected response values established by the
test and assert the translated payload preserves them end to end.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 1b09ea38-a969-4a5c-ab70-5a22ccea7478

📥 Commits

Reviewing files that changed from the base of the PR and between 6e85060 and 4ad28e9.

📒 Files selected for processing (2)
  • crates/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • crates/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/src/gateway/mod.rs
  • crates/cli/tests/switchyard_process_e2e.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/cli/tests/switchyard_process_e2e.rs
🔇 Additional comments (2)
crates/cli/src/gateway/mod.rs (1)

84-84: LGTM!

Also applies to: 243-243

crates/cli/tests/switchyard_process_e2e.rs (1)

46-46: LGTM!

Also applies to: 90-102, 251-251, 269-285, 290-300, 301-307, 332-346, 347-359

@bbednarski9

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 8c971a9 into NVIDIA:release/0.6 Jul 20, 2026
35 of 36 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:S PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants