Skip to content

feat(server): bind guarded streams to HTTP - #2436

Merged
Pouyanpi merged 11 commits into
developfrom
pouyanpi/transparent-proxy-streaming-http-3
Oct 13, 2026
Merged

Pouyanpi merged 11 commits into
developfrom
pouyanpi/transparent-proxy-streaming-http-3

Conversation

@Pouyanpi

@Pouyanpi Pouyanpi commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds an injected streaming HTTP lifecycle bridge around the provider-neutral runner, including response validation and reliable source closure.

What Changed

  • Validates SSE media type, inspected identity encoding, and stream framing before headers.
  • Applies one Content-Encoding rule to buffered and streaming bodies: duplicate headers and comma lists form one coding list, and a body is unencoded only if every coding is identity.
  • Requires exactly one Content-Type on guarded requests, inspected buffered responses, and inspected streams; duplicates are rejected even when they agree.
  • Adds an optional close callback to StreamingHttpResponse that releases the upstream resource independently of the body iterator. A bare async-generator body must provide it; self-owning iterators may expose an idempotent aclose.
  • Closes the iterator and the resource with shared bounded, shielded cleanup on every path, including normal EOF and disconnect during reads or checks. Cleanup failures are logged without replacing the primary outcome.
  • Reuses the existing kernel response-header policy and buffered renderer for provider HTTP errors.
  • Preserves valid uninspected response lengths, detects relay length mismatches, and removes body-dependent headers when guarded output can change.
  • Exercises mixed endings, BOMs, byte preservation, and truncation through the real OpenAI stream classifier.

Review Notes

Review pre-header validation, framing, transparent relay, and resource ownership. The Content-Encoding and Content-Type rules also change buffered behavior: gzip followed by identity, and duplicate Content-Type headers, are now rejected. Dispatch remains injected; concrete outbound HTTP and application client lifecycle remain separate work.

AI Assistance

  • No AI tools were used.
  • AI tools were used (Codex and Claude Code assisted with implementation, regression tests, review probes, and PR drafting). Human review of the new diff is pending.

Checklist

  • I've read the CONTRIBUTING guidelines.
  • This is maintainer-directed work.
  • The PR title follows the project commit convention.
  • Applicable declarations and tests are included.
  • Generated changelog files were not edited manually.
  • Automated and human review comments are addressed or answered.
  • The responsible reviewer or team is mentioned.

Stack Position

Part 3 of 4.

Stack Context

Adds OpenAI Chat streaming above the merged buffered implementation on develop in four parts: provider-neutral execution, typed OpenAI stream declarations and handwritten hooks, injected HTTP lifecycle handling, and integration with the buffered request pipeline.

The open chain is develop → #2434 → #2435 → #2436 → #2437. The buffered foundation is merged; #2412 is closed.

Python declarations own field and event policy. Hooks are handwritten and created for each stream by the integration. The final PR exports the bound buffered and streaming policy for inspection; export never constructs runtime components. Concrete outbound HTTP, deployment configuration, and replacement execution land separately.

Review each PR against its listed base branch.

Order PR Branch Base
1 #2434 pouyanpi/transparent-proxy-streaming-kernel-1 develop
2 #2435 pouyanpi/openai-chat-streaming-contract-2 pouyanpi/transparent-proxy-streaming-kernel-1
3 #2436 pouyanpi/transparent-proxy-streaming-http-3 pouyanpi/openai-chat-streaming-contract-2
4 #2437 pouyanpi/openai-chat-streaming-integration-4 pouyanpi/transparent-proxy-streaming-http-3

Summary by CodeRabbit

  • New Features
    • Added support for relaying server-sent event streams over HTTP, with optional safeguards that inspect and limit streamed content.
    • Streaming responses preserve their event data and are closed cleanly when finished or interrupted.
  • Bug Fixes
    • Improved HTTP response handling by filtering connection-specific headers and ensuring downstream framing reflects the response body.
    • Invalid or unsafe streams are rejected rather than relayed.

@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from e97105a to 3dd8fea Compare October 6, 2026 11:05
@Pouyanpi Pouyanpi added this to the v0.25.0 milestone Oct 6, 2026
@Pouyanpi Pouyanpi self-assigned this Oct 6, 2026
@Pouyanpi Pouyanpi removed the status: needs triage New issues that have not yet been reviewed or categorized. label Oct 6, 2026
@Pouyanpi
Pouyanpi marked this pull request as ready for review October 6, 2026 11:06
@Pouyanpi Pouyanpi added the status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile). label Oct 6, 2026
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from 3dd8fea to bd3fade Compare October 6, 2026 15:22
@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[High impact] Adds streaming HTTP response handling to guarded provider operations.

The previous finding about delayed metadata and keepalives remains unresolved and should be addressed before merging.

Findings

  1. P1 Metadata holds back keepalives ▶
Fix with agent prompt
### Issue 1
nemoguardrails/server/experimental/_guarded_stream.py:261-274
Data-bearing metadata enters `pending` even when no unchecked text is waiting. Later keepalives are then held until another text window passes or the provider closes the connection.

If the provider sends an initial role event and keeps sending keepalives while generating its first token, those keepalives never reach the client. A healthy stream can time out. Provider errors and `[DONE]` also remain queued if the upstream connection stays open.

Release checked metadata promptly when no text is pending. At completion, flush pending text through the checker without waiting for the connection to close.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds an injected HTTP bridge for guarded provider streams.

  • Guarded HTTP calls can relay open streams or check their events.
  • HTTP responses close stream resources when sending ends.
  • Guarded HTTP paths reject ambiguous content headers.

Pouyanpi accepted the identity-encoding limitation because inspection needs readable bytes. Client negotiation or rejection remains a separate follow-up.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Prepared request] --> B[Injected dispatch]
  B --> C{Response kind}
  C -->|Buffered error| D[Shared buffered renderer]
  C -->|Open stream| E{Headers acceptable?}
  E -->|No| F[Close upstream and return error]
  E -->|Yes| G{Inspect output?}
  G -->|No| H[Relay bytes and check length]
  G -->|Yes| I[Check text before release]
  H --> J[Close upstream on completion or disconnect]
  I --> J
Loading

Reviews (10) · Last reviewed commit: "test(server): cover streaming HTTP valid..." · Reviewed by Greptile

Comment thread nemoguardrails/server/experimental/_streaming_http.py
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from bd3fade to 5814177 Compare October 6, 2026 17:30
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from 5814177 to 50a025d Compare October 7, 2026 09:00
@greptile-apps

This comment has been minimized.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA-NeMo/Guardrails/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 6ff5c615-0638-4baf-bfed-9c773cbaf822

📥 Commits

Reviewing files that changed from the base of the PR and between 969832f and fe7408e.


📒 Files selected for processing (3)
  • nemoguardrails/server/experimental/_http_kernel.py
  • nemoguardrails/server/experimental/_streaming_http.py
  • tests/server/experimental/test_streaming_http.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.



📝 Walkthrough

Walkthrough

The PR adds an experimental HTTP streaming adapter. It validates streamed responses, relays or guards their bodies, filters headers, and closes response bodies and provider sources. It also updates shared response header filtering.

Changes

Streaming HTTP response handling

Layer / File(s) Summary
Header filtering and framing
nemoguardrails/server/experimental/_http_kernel.py, nemoguardrails/server/experimental/_streaming_http.py, tests/server/experimental/test_streaming_http.py
Shared response rendering filters hop-by-hop headers and omits upstream Content-Length before downstream framing. Streaming header helpers filter body-dependent headers when inspection may modify the body. Tests cover header filtering and framing validation.
Streaming dispatch and body lifecycle
nemoguardrails/server/experimental/_streaming_http.py, tests/server/experimental/test_streaming_http.py
The adapter validates dispatch results and SSE framing, then relays uninspected streams or guards streams when a buffering policy is configured. It enforces declared content lengths and closes response bodies and provider sources. Tests cover dispatch failures, stream validation, errors, cancellation, and cleanup.

Priority: ➖ Normal

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant execute_streaming_http
  participant dispatch
  participant checker
  Client->>execute_streaming_http: Submit request
  execute_streaming_http->>dispatch: Dispatch prepared request
  dispatch-->>execute_streaming_http: Return streaming response
  opt Buffering policy configured
    execute_streaming_http->>checker: Guard provider stream
  end
  execute_streaming_http-->>Client: Return streaming response
Loading

Merge Risk: ⚪ Minimal · up to fe740

No confirmed issue prevents merging after normal checks.

Pre-merge checks | Passed 5 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 80 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Test Results For Major Changes Passed The PR is major: it adds a streaming HTTP feature and 857 changed lines, including 602 test lines. The description documents testing information for headers, framing, closure, limits, lengths, SSE par…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: integrating guarded provider streams with the HTTP server lifecycle.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from fe7408e to 9112df3 Compare October 9, 2026 14:17
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from 9112df3 to 6668113 Compare October 9, 2026 15:22
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from 6668113 to aa07f76 Compare October 12, 2026 13:21
@Pouyanpi

Copy link
Copy Markdown
Collaborator Author

@greptileai the stream index doesn't break export: stream export only exists from #2437, and its export commit adds minimum/maximum to the format and regenerates the artifact. contract_export --check passes at every #2437 commit. Could you re-check?

@tgasser-nv tgasser-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment thread nemoguardrails/server/experimental/_streaming_http.py Outdated
Base automatically changed from pouyanpi/openai-chat-streaming-contract-2 to develop October 13, 2026 10:03
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Make the streaming response the owner of the upstream body. It closes
the body iterator and the source in a shielded cleanup, so a client
disconnect or send failure before the first chunk no longer leaks the
provider connection. Reject a non-user input message before dispatch
instead of failing after the response starts.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
An inspected stream may withhold events, emit a provider-native error, or
drop an unfinished tail, so validators computed over the provider body no
longer describe the bytes sent downstream. Remove ETag, Content-MD5,
Digest, Content-Digest, and Repr-Digest alongside Content-Length whenever
the stream may change. Uninspected relays keep every end-to-end header.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Clients such as the OpenAI SDK send Accept-Encoding: gzip, and inspected
streams reject a provider stream that is not identity-encoded. A
compressing provider therefore turned every inspected streaming request
into a proxy error.

When the stream is inspected, replace Accept-Encoding with identity
before dispatch, matching buffered output inspection. Uninspected relays
forward the client's preference unchanged, and a still-encoded inspected
stream remains rejected.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Buffered requests and responses read only the last Content-Encoding header,
so "gzip" followed by "identity" passed as unencoded, while streams combined
every header but rejected an empty value. Both paths now share one rule:
duplicate headers and comma lists form one coding list, empty elements are
ignored, and the body is unencoded only when every coding is identity.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Closing the body iterator does not prove the upstream connection is closed.
A client disconnect can cancel an async generator's own cleanup part-way, and
later aclose() calls cannot resume it. StreamingHttpResponse now carries an
optional close callback that releases the upstream resource directly.
Iterators that own their resource may keep exposing aclose(); a bare async
generator must supply the callback and is otherwise rejected.

Dispatch owns resources until it returns the response and must clean up if
construction fails. From then on the executor owns cleanup on every path,
including rejection and disconnect before iteration. Closing must be
idempotent; each attempt is bounded and failures are logged.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
The streaming executor treated any exception from dispatch as an upstream
failure, so a bug in a dispatcher or in our own code was reported to the
client and operators as a provider outage. It now catches only
HttpDispatchFailed, like the buffered path; other exceptions propagate.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Adds cases for invalid StreamingHttpResponse values, non-byte chunks in an
uninspected relay, non-positive byte limits, and an unsupported dispatch
result. No runtime behavior changes.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Comment thread nemoguardrails/server/experimental/_guarded_stream.py
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-http-3 branch from 78cd081 to ef094d6 Compare October 13, 2026 10:08
@Pouyanpi
Pouyanpi merged commit f09882a into develop Oct 13, 2026
20 checks passed
@Pouyanpi
Pouyanpi deleted the pouyanpi/transparent-proxy-streaming-http-3 branch October 13, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: L status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants