feat(translation): preserve raw stream events - #192
Conversation
|
@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 |
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
|
@bbednarski9 Can you have Claude run the I'm not sure if those are relevant. And more minor and style nits. |
0dd88a1 to
aa0a311
Compare
|
Addressed the three remaining review findings in separate commits:
Validation: workspace Clippy with warnings denied passed; all workspace tests excluding |
|
Addressed the algebraic-type review in 445f999: |
445f999 to
98bdfdc
Compare
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
…replay Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
…al event Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Keep preservation on TranslationEngine, the API consumed by NVIDIA/NeMo-Relay#586, and remove the unused built-in convenience decoder. Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
98bdfdc to
8ef2ad3
Compare
What
This PR adds a composed streaming-response event to Switchyard so a host can send a provider response through
libsy::Algorithm::run_streamwithout discarding provider-specific JSON fields.The implementation:
LlmResponseChunkprovider-neutral;LlmResponseStreamto carryLlmResponseStreamEvent;ProviderStreamEvent { source, raw }preservation;TranslationEngine, the API used by the concrete downstream Relay consumer;rawwhen the source and target formats match;normalizedchunks when the target format differs;The fields are intentionally accessed through constructors and accessors. Replacing normalized content creates a normalized-only event and drops preservation, preventing stale provider JSON from being replayed after an algorithm changes the semantic content.
This 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 LlmResponseStreamEvent<br/>preservation + normalized chunks Host-->>Libsy: Respond to CallLlm with event stream Note over Libsy: Algorithms inspect or aggregate<br/>provider-neutral normalized chunks Libsy-->>Host: ReturnToAgent(event stream) alt Caller format matches provider format Host-->>Caller: Replay preserved raw JSON else Caller format differs Host->>Host: Encode normalized chunks<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 } ] }Switchyard normalizes the semantic content to approximately:
That normalized chunk has no representation for
system_fingerprint. The composed stream item carries both:Algorithms consume the normalized meaning.
switchyard-translationalone interprets preservation: it replays the parsed raw JSON for same-format output and ignores it when encoding a different protocol.The concrete downstream consumer is NVIDIA/NeMo-Relay#586. Its streaming adapter calls
TranslationEngine::decode_stream_eventfor each host-dispatched provider event andTranslationEngine::encode_stream_eventafterReturnToAgent; a Switchyard backend, server route, or PyO3 binding is therefore not required for this contract.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.LlmResponseChunkremains provider-neutral. Preservation is composed beside normalized chunks at theLlmResponseStreamEventboundary.LlmResponseStreamitem type changes fromLlmResponseChunktoLlmResponseStreamEvent, so downstream stream producers and consumers must wrap or unwrap normalized chunks.replace_normalizedAPI enforces that behavior.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 doc --workspace --no-depscargo test -p switchyard-protocolcargo test -p switchyard-translationcargo test -p switchyard-libsycargo test -p switchyard-llm-clientcargo test -p switchyard-servercargo test --workspace --exclude switchyard-pyThe same-format helper-path test sends an OpenAI Chat stream event containing
system_fingerprintthroughdecode_streamandencode_streamand verifies exact parsed-JSON replay. Cross-format tests verify that normalized content is encoded instead.The 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 between
preservationandnormalized, and on theLlmResponseStreamEventhost/algorithm contract 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.Preservation decoding is exposed through
TranslationEngineand the stream helper used byswitchyard-llm-client. Both encoding surfaces delegate to the same shared preservation dispatcher. Provider codecs handle only normalized chunks, including when the engine uses a custom registered codec.Summary by CodeRabbit
New Features
Bug Fixes