feat(translation): preserve raw stream events - #192
Open
bbednarski9 wants to merge 1 commit into
Open
Conversation
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
bbednarski9
marked this pull request as ready for review
July 30, 2026 00:24
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
WalkthroughChangesProvider Event Streaming
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/switchyard-translation/tests/stream_translation.rs (1)
16-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd concise comments documenting the preservation contracts under test.
crates/switchyard-translation/tests/stream_translation.rs#L16-L69: note that equality is for the parsed JSON value replayed to the same provider format.crates/switchyard-translation/tests/stream_translation.rs#L71-L97: note that cross-format encoding must discard provider-specific fields and use normalized events.As per coding guidelines, Rust changes require concise comments for important test behavior.
🤖 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/switchyard-translation/tests/stream_translation.rs` around lines 16 - 69, Add concise comments documenting the two preservation contracts in stream_translation.rs: at lines 16-69, clarify that same-format replay compares the parsed JSON value emitted back to the same provider format; at lines 71-97, clarify that cross-format encoding uses normalized events and discards provider-specific fields.Source: Coding guidelines
crates/protocol/src/stream.rs (1)
78-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the raw-replay versus normalized-routing invariant.
crates/protocol/src/stream.rs#L78-L101: explain recursive error propagation through normalized children.crates/switchyard-translation/src/engine.rs#L298-L316: document same-format raw replay versus cross-format normalized encoding.crates/switchyard-translation/src/codecs/anthropic/stream.rs#L129-L137: annotate the ProviderEvent routing branch.crates/switchyard-translation/src/codecs/openai_chat/stream.rs#L157-L165: annotate the ProviderEvent routing branch.crates/switchyard-translation/src/codecs/responses/stream.rs#L157-L165: annotate the ProviderEvent routing branch.As per coding guidelines, Rust changes require concise comments for non-obvious private helpers and complex routing logic.
🤖 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/protocol/src/stream.rs` around lines 78 - 101, Document the raw-replay versus normalized-routing invariant with concise comments: in crates/protocol/src/stream.rs lines 78-101, explain that push_checked_chunk recursively processes normalized ProviderEvent children and propagates their errors; in crates/switchyard-translation/src/engine.rs lines 298-316, describe raw replay for same-format responses versus normalized encoding across formats; and annotate the ProviderEvent routing branches in crates/switchyard-translation/src/codecs/anthropic/stream.rs lines 129-137, crates/switchyard-translation/src/codecs/openai_chat/stream.rs lines 157-165, and crates/switchyard-translation/src/codecs/responses/stream.rs lines 157-165.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.
Inline comments:
In `@crates/protocol/src/stream.rs`:
- Around line 103-108: The documentation overstates preservation as exact or
lossless replay; describe it as retaining and replaying parsed JSON values only.
Update the documentation in crates/protocol/src/stream.rs lines 103-108,
crates/switchyard-translation/src/codecs/stream.rs line 217, and
crates/switchyard-translation/src/engine.rs lines 247 and 264-268 to use
parsed-event, parsed-value, and structural JSON-value replay wording, while
preserving the existing API behavior.
---
Nitpick comments:
In `@crates/protocol/src/stream.rs`:
- Around line 78-101: Document the raw-replay versus normalized-routing
invariant with concise comments: in crates/protocol/src/stream.rs lines 78-101,
explain that push_checked_chunk recursively processes normalized ProviderEvent
children and propagates their errors; in
crates/switchyard-translation/src/engine.rs lines 298-316, describe raw replay
for same-format responses versus normalized encoding across formats; and
annotate the ProviderEvent routing branches in
crates/switchyard-translation/src/codecs/anthropic/stream.rs lines 129-137,
crates/switchyard-translation/src/codecs/openai_chat/stream.rs lines 157-165,
and crates/switchyard-translation/src/codecs/responses/stream.rs lines 157-165.
In `@crates/switchyard-translation/tests/stream_translation.rs`:
- Around line 16-69: Add concise comments documenting the two preservation
contracts in stream_translation.rs: at lines 16-69, clarify that same-format
replay compares the parsed JSON value emitted back to the same provider format;
at lines 71-97, clarify that cross-format encoding uses normalized events and
discards provider-specific fields.
🪄 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: CHILL
Plan: Enterprise
Run ID: f57ca300-512e-41f4-92bf-4be3ad0a3101
📒 Files selected for processing (7)
crates/protocol/src/stream.rscrates/switchyard-translation/src/codecs/anthropic/stream.rscrates/switchyard-translation/src/codecs/openai_chat/stream.rscrates/switchyard-translation/src/codecs/responses/stream.rscrates/switchyard-translation/src/codecs/stream.rscrates/switchyard-translation/src/engine.rscrates/switchyard-translation/tests/stream_translation.rs
Comment on lines
+103
to
+108
| /// One streaming event carried between a host and an algorithm. | ||
| /// | ||
| /// Normalized variants expose provider-neutral meaning. [`ProviderEvent`](Self::ProviderEvent) | ||
| /// additionally retains exact source JSON for a lossless same-format round trip while keeping | ||
| /// normalized children available to algorithms and cross-format encoders. | ||
| /// `switchyard-translation` re-exports this type as `ConversationStreamEvent`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe preservation as parsed-JSON-value replay, not lossless replay.
The PR contract excludes original SSE bytes, comments, IDs, whitespace, and key ordering, so “exact” and “lossless” can mislead API consumers.
crates/protocol/src/stream.rs#L103-L108: state thatrawretains the parsed JSON value.crates/switchyard-translation/src/codecs/stream.rs#L217-L217: replace “exact source JSON” with parsed-event wording.crates/switchyard-translation/src/engine.rs#L247-L247: qualify retained JSON as a parsed value.crates/switchyard-translation/src/engine.rs#L264-L268: qualify replay as structural JSON-value replay.
Based on PR objectives, preservation is limited to parsed JSON values rather than original SSE representation.
📍 Affects 3 files
crates/protocol/src/stream.rs#L103-L108(this comment)crates/switchyard-translation/src/codecs/stream.rs#L217-L217crates/switchyard-translation/src/engine.rs#L247-L247crates/switchyard-translation/src/engine.rs#L264-L268
🤖 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/protocol/src/stream.rs` around lines 103 - 108, The documentation
overstates preservation as exact or lossless replay; describe it as retaining
and replaying parsed JSON values only. Update the documentation in
crates/protocol/src/stream.rs lines 103-108,
crates/switchyard-translation/src/codecs/stream.rs line 217, and
crates/switchyard-translation/src/engine.rs lines 247 and 264-268 to use
parsed-event, parsed-value, and structural JSON-value replay wording, while
preserving the existing API behavior.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This PR adds a provider-event envelope to Switchyard's streaming protocol so a host can send a provider response through
libsy::Algorithm::run_streamwithout discarding provider-specific JSON fields.The implementation:
LlmResponseChunk::ProviderEvent { source, raw, normalized }toswitchyard-protocol;TranslationEngineand the built-in stream helpers;rawwhen the source and target formats match;normalizedchildren when the target format differs;LlmResponse::into_aggandResponseAccumulatorto recursively fold normalized children while preserving typed decode and upstream-stream errors; andThis is intentionally limited to the streaming translation contract.
Lifecycle
sequenceDiagram participant Provider participant Host as Host / Relay participant Libsy as libsy run_stream participant Caller Provider-->>Host: Raw JSON stream event<br/>text delta + provider extension Host->>Host: Decode to ProviderEvent<br/>{ source, raw, normalized } Host-->>Libsy: Respond to CallLlm with stream Note over Libsy: Algorithms can aggregate or inspect<br/>the normalized meaning Libsy-->>Host: ReturnToAgent(stream) alt Caller format matches provider format Host-->>Caller: Replay the preserved raw JSON event else Caller format differs Host->>Host: Encode normalized children<br/>into caller format Host-->>Caller: Cross-format stream event endWhy
run_streamdeliberately gives the embedding host control of provider dispatch. The host fulfills eachCallLlm, but the resulting stream still passes back through libsy so an algorithm can inspect it, aggregate it, or select it as the final response.Before this change, decoding a provider event produced only provider-neutral chunks. That is sufficient for cross-format translation, but it cannot faithfully replay a same-format stream after the libsy round trip.
For example, an OpenAI-compatible provider may emit:
{ "id": "chatcmpl-1", "object": "chat.completion.chunk", "system_fingerprint": "fp_abc", "choices": [ { "index": 0, "delta": {"content": "Hello"}, "finish_reason": null } ] }Without the envelope, Switchyard normalizes this to approximately:
When that chunk returns from libsy, the encoder can reconstruct the text delta but has no representation for
system_fingerprint; the field is silently lost. WithProviderEvent, the normalizedTextDeltaremains available to algorithms while the complete parsed JSON value remains available for same-format replay.This enables library-first integrations such as NeMo Relay to:
CallLlmRequest::respond;ReturnToAgent; andFor cross-format routes, only fields represented by the neutral IR are translated. In the example above, OpenAI
system_fingerprintis intentionally absent from an Anthropic Messages output because Anthropic has no corresponding field.Boundaries and compatibility
serde_json::Value, not for the original SSE bytes. Comments, SSEidfields, whitespace, and JSON key ordering are not retained by this envelope.LlmResponseChunkmatches to handleProviderEvent.normalizedchildren.Closes: N/A — this is an integration-discovered library contract gap.
How tested
uv run ruff check .clean — not applicable; no Python changeduv run mypy switchyardclean — not applicable; no Python changeduv run pytest tests/green — not applicable; no Python changedRust validation:
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warningscargo test -p switchyard-translationcargo test --workspace --exclude switchyard-pyThe non-PyO3 workspace suite passes. A local
cargo test --workspaceattempt reachedswitchyard-pybut could not link the macOS test artifact because the local linker did not resolve Python symbols; no Rust test failed before that link step.Integration validation used NeMo Relay as the host-side dispatcher with no Switchyard service or
switchyard-llm-client. Buffered and streaming routes were exercised across OpenAI Chat, OpenAI Responses, and Anthropic Messages caller APIs, using NVIDIA Inference Hub, Anthropic, and Ollama targets.Checklist
snake_caseof the primary class. — not applicable; Rust-only changeswitchyard/__init__.py.__all__if intended for downstream use. — not applicable; Rust crate API--helpupdated if customer-facing surface changed. — not required; no CLI/configuration changeSigned-off-by: Your Name <email>) per the DCO.Notes for reviewers
Please focus on the ownership boundary of
rawversusnormalizedand on whether a publicProviderEventvariant is the preferred representation before Switchyard 0.2 stabilizes. The same-format path never reconstructs an event from the normalized IR; the cross-format path never copies unknown provider fields into the target protocol.The direct built-in helper surface and
TranslationEnginesurface are both covered so callers do not get different preservation semantics depending on which public translation entry point they use.Summary by CodeRabbit
New Features
Bug Fixes