Skip to content

feat(server): execute guarded provider streams - #2434

Merged
Pouyanpi merged 16 commits into
developfrom
pouyanpi/transparent-proxy-streaming-kernel-1
Oct 13, 2026
Merged

Pouyanpi merged 16 commits into
developfrom
pouyanpi/transparent-proxy-streaming-kernel-1

Conversation

@Pouyanpi

@Pouyanpi Pouyanpi commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds provider-neutral SSE parsing, event classification, bounded buffering, and guarded stream execution. Content-bearing events are checked before release, and allowed events retain their original bytes.

What Changed

  • Declares stream roles, shape coverage, adapters, and declarative event classifiers.
  • Frames mixed SSE line endings incrementally and preserves raw bytes. One BOM at stream start is ignored; a BOM at the start of a later line is rejected, because SSE clients disagree on it (openai-node strips it, the standard does not).
  • Rejects unfinished field blocks and drops unfinished comment tails; error hooks must encode complete SSE blocks.
  • Bounds events and unchecked pending bytes, verifies snapshots, and rejects modification requests.
  • Checks the original wire text and fails closed if validation would change, drop, or reorder the guarded text.
  • Matches discriminators by exact type and value, and treats a missing member differently from an explicit null.
  • Relays lone surrogates unchanged, as clients receive them; a checker that cannot handle them fails closed.
  • Keeps the async-generator interface. Iterator and resource cleanup is bounded and shielded; cleanup failures are logged without replacing the stream's outcome.

Review Notes

Review release ordering, SSE ambiguity, bounds, byte preservation, and cancellation and cleanup. HTTP dispatch, provider grammar, and request preparation are supplied by later PRs.

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 1 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 inspecting provider-streamed responses before releasing buffered content, with configurable chunk and context sizes.
    • Unsafe or uninspectable streamed content can be withheld, while provider errors and stream-processing failures are handled as distinct outcomes.
    • Added support for parsing streamed events across varied chunk boundaries and validating provider event formats.

@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.82425% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...oguardrails/server/experimental/provider/stream.py 98.87% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-kernel-1 branch from 4ec5c7f to f4a6606 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 status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile). and removed status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile). labels Oct 6, 2026
@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous private-text logging issue is fixed, and no new actionable issue was found.

Summary

Adds provider-neutral guarded streaming that checks content before releasing the original SSE bytes.

  • SSE events keep their original bytes when streams arrive in pieces.
  • Provider adapters classify events using declared stream shapes.
  • Guarded streams check text before releasing original events.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Provider bytes] --> B[Frame SSE events]
    B --> C[Classify and check event shape]
    C --> D[Buffer guarded text]
    D --> E{Checker allows text?}
    E -- Yes --> F[Release original event bytes]
    E -- No --> G[Send provider-shaped error]
    B -- Invalid stream --> G
    F --> H[Bounded stream cleanup]
    G --> H
Loading

Reviews (9) · Last reviewed commit: "fix(server): log stream failures by type..." · Reviewed by Greptile

Comment thread nemoguardrails/server/experimental/provider/sse.py Outdated
Comment thread nemoguardrails/server/experimental/provider/sse.py Outdated
Comment thread nemoguardrails/server/experimental/_guarded_stream.py Outdated
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-kernel-1 branch 2 times, most recently from 9052700 to cb9fb48 Compare October 8, 2026 17:52
Base automatically changed from pouyanpi/openai-chat-buffered-integration-4 to develop October 9, 2026 13:16
@Pouyanpi
Pouyanpi added this pull request to stack #2445 October 9, 2026 13:16
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Parse inspected streams line by line so mixed CR, LF, and CRLF endings
and terminators split across chunks frame correctly in linear time.
Strip one initial BOM for field parsing while keeping it in raw bytes.
Release complete SSE blocks only, drop unfinished comment tails, and
reject unfinished field blocks as truncated provider streams.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
@Pouyanpi
Pouyanpi force-pushed the pouyanpi/transparent-proxy-streaming-kernel-1 branch from cb9fb48 to 558d602 Compare October 9, 2026 13:22
@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: 78bd4143-092c-4fcc-818f-56da00e60ccc

📥 Commits

Reviewing files that changed from the base of the PR and between 4512d08 and 558d602.


📒 Files selected for processing (9)
  • nemoguardrails/server/experimental/_content_checker.py
  • nemoguardrails/server/experimental/_guarded_stream.py
  • nemoguardrails/server/experimental/provider/sse.py
  • nemoguardrails/server/experimental/provider/stream.py
  • nemoguardrails/server/experimental/provider/stream_classifier.py
  • tests/server/experimental/test_content_checker.py
  • tests/server/experimental/test_guarded_stream.py
  • tests/server/experimental/test_sse.py
  • tests/server/experimental/test_stream_classifier.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

This change adds configuration for buffering provider streams, parses and classifies server-sent events, and checks buffered text before releasing its original events. It also adds validation for stream contracts, limits, snapshots, failure outcomes, and source cleanup.

Changes

Guarded provider stream inspection

Layer / File(s) Summary
Stream buffering policy
nemoguardrails/server/experimental/_content_checker.py, tests/server/experimental/test_content_checker.py
Adds validated chunk and context sizes, a release setting, and optional buffering configuration on ContentInspectionPolicy. Tests cover valid and invalid policy values.
Byte-preserving SSE parsing
nemoguardrails/server/experimental/provider/sse.py, tests/server/experimental/test_sse.py
Adds event framing across byte chunks, preserves raw event bytes, exposes parsed data and event fields, and handles size limits and truncated input. Tests cover line endings, BOMs, comments, and large inputs.
Provider stream contracts and classification
nemoguardrails/server/experimental/provider/stream.py, nemoguardrails/server/experimental/provider/stream_classifier.py, tests/server/experimental/test_stream_classifier.py
Adds event roles, projection contracts, adapter protocols, and rule-based classification. Tests cover recognized events, text selection, discriminator types, and contract mismatches.
Guarded stream buffering and release
nemoguardrails/server/experimental/_guarded_stream.py, tests/server/experimental/test_guarded_stream.py
Adds event validation, buffered text-window checks, snapshot history checks, and outcome rendering. Tests cover event release, blocked output, error handling, limits, and source closure.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Source as Async byte source
  participant Guard as guard_provider_stream
  participant Parser as iter_sse_events
  participant Adapter as ProviderStreamAdapter
  participant Checker as ContentChecker
  participant Renderer as StreamOutcomeRenderer
  Source->>Guard: yield byte chunks
  Guard->>Parser: pass source chunks
  Parser-->>Guard: return parsed SSE events
  Guard->>Adapter: classify and validate events
  Adapter-->>Guard: return guarded stream events
  Guard->>Checker: check buffered text windows
  Checker-->>Guard: return check result
  Guard->>Renderer: render blocked or failed outcome
  Guard-->>Source: close async source
Loading

Merge Risk: ⚪ Minimal · up to 558d6

This change adds experimental guarded stream parsing and release logic that nothing yet uses, and no concrete merge-blocking issue was found.

Pre-merge checks | Passed 5 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 103 functions across 9 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 contains major new stream execution and parsing features, but the description includes testing information: it states that applicable tests are included and notes that regression tests were par…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: adding guarded provider-stream execution in the server.

  • 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.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Output checks receive one projected user input subject as context in both
buffered and streaming execution. Document that this is a provider-neutral
execution invariant rather than a restriction on provider conversation
roles, and that relaxing it needs matching bindings, checker semantics, and
execution tests, not only provider or profile declarations.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
The SSE standard ignores one byte order mark at stream start. The parser
treated a later line that starts with a BOM as an unknown field, so its
event had no data and was released as opaque metadata without text
checks. Some clients, including openai-node, strip a BOM at later line
starts and read the line as a data field, so a provider could deliver
text that output rails never inspected.

Keep one stream-leading BOM. Reject any other line that starts with a BOM,
including BOM-only lines and a second leading BOM, as an ambiguous event
that fails the inspected stream closed. The check runs on complete lines,
so it is independent of chunk boundaries and line endings. Uninspected
relays are not parsed and are unaffected.

Tests cover every chunk split, LF, CR, and CRLF endings, BOM-only lines,
comments, unfinished tails, withheld events in the guarded stream, and a
differential check that the OpenAI Python SDK decoder never yields data
that this parser did not parse.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
JSON can encode an unpaired UTF-16 surrogate as an escape, and the JSON
parser also accepts its CESU-8 bytes. Such text cannot be re-encoded as
UTF-8, so the guarded stream failed with a generic processing error
while counting text bytes.

Treat guarded or snapshot text with an unpaired surrogate as an
unsupported provider stream: no output check runs, pending events are
withheld, and the error is rendered as a stream shape failure.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
A transparent proxy should not enforce Unicode validity that the provider
and client do not. JSON escapes can produce lone surrogates, for example
half of a pair split across deltas, and clients receive them as sent. The
stream path now counts their bytes without failing, checks the text, and
relays it unchanged. A checker that cannot handle such text still fails
closed as a check failure. This replaces the earlier rejection.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
The Python SDK never strips a BOM, so comparing with it cannot catch the
line-leading BOM differential. A small reader modelled on openai-node 7.31.0
strips a BOM at every line start; accepted streams must expose no data to it
that this parser did not parse. Disabling the BOM rejection fails these cases.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
The classifier read guarded text from the validated projection, while the
runner relays the original event bytes. A validator that changed, dropped or
reordered the selected text would let the checker approve one string while
the client received another. Text is now resolved from the raw payload before
validation and compared afterwards; any difference fails closed. Null,
missing and empty text keep their existing meaning.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Rule construction compared discriminators with Python equality, so rules for
true and 1 were rejected as duplicates although runtime matching treats them
as different values. Duplicate detection now uses the same exact type and
value as matching. Array selection also stops treating a missing member as an
explicit null.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
The guarded stream closed its source without a time limit, so a provider body
whose close stalled could hang an otherwise complete response. Iterator and
resource cleanup now share one helper: each attempt is shielded and bounded,
and timeouts or ordinary close failures are logged instead of replacing the
stream's outcome. Close failures were previously raised; this is deliberate,
because cleanup must not mask a block, an upstream failure or a cancellation.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
…celled

If closing the iterator raised cancellation, the separate resource closer was
skipped and the upstream connection could stay open. The resource closer now
runs in a finally block, so it is attempted however iterator cleanup ends.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>

@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, just a few items to address before merging:

  1. Could you add tests to get the patch coverage up?
  2. There are quite a few features the OpenAI Chat binding doesn't use. StreamArraySelection, event_type_field, TEXT_SNAPSHOT with the history check and missing_text_value. Can you add tests to cover these while we're OpenAI only?

Comment thread nemoguardrails/server/experimental/_guarded_stream.py
Stream failures reached the client as a native error event but left no
record for operators. Each failure outcome is now logged once, where it is
encoded: projection and framing rejections and unsupported modification
requests as warnings with their stage and reason, upstream failures as
warnings with the exception type, and checker or processing failures as
errors with the exception attached. Blocks are ordinary checker decisions
and are not logged here. Reasons describe shapes and never echo provider
text.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
The OpenAI Chat binding exercises only part of the shared stream kernel.
Synthetic rules now cover data-less events, the SSE event-type
discriminator, exactly-one array selection, dotted match keys, missing
text handling, text paths that do not reach a string, snapshot history
beyond the verification limit, non-byte provider chunks, and every
classifier definition and contract consistency check. No runtime
behavior changes.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Comment thread nemoguardrails/server/experimental/_guarded_stream.py Outdated
Failure messages and tracebacks can carry provider or checked text, such
as a duplicate JSON member name or a checker error. Log only the outcome
type, stage and exception type.

Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
@Pouyanpi
Pouyanpi merged commit eee61aa into develop Oct 13, 2026
18 checks passed
@Pouyanpi
Pouyanpi deleted the pouyanpi/transparent-proxy-streaming-kernel-1 branch October 13, 2026 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: XL 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