fix(presidio): stream SSE output incrementally instead of buffering the whole response - #31503
Conversation
|
|
Greptile SummaryThis PR changes Presidio streaming output handling to rewrite chunks incrementally. The main changes are:
Confidence Score: 5/5The streaming rewrite appears merge-safe with focused test coverage around incremental Presidio masking, unmasking, ordering, and error handling. The change is scoped to Presidio streaming output handling and includes targeted regressions for the main edge cases introduced by incremental rewriting, including split tokens, multi-choice streams, tool/function arguments, passthrough ordering, buffer caps, and failure behavior.
What T-Rex did
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 4 · PR risk: 0/10 |
07423b9 to
93b966a
Compare
|
Addressed the 4/5 finding: in the apply_to_output path, buffered masked content is now flushed before any non-chat passthrough event (raw bytes or a /v1/responses completion), so a client never sees stream completion ahead of the final masked text. This mirrors the unmask path, which already flushed-before-passthrough. Added a regression test (test_mask_streaming_flushes_buffered_content_before_passthrough_event) that fails without the flush. |
93b966a to
2983514
Compare
|
Pushed fixes for the three open streaming findings. The forced-flush window ( Each fix has a regression that fails on the prior code: |
2983514 to
83ec037
Compare
|
Addressed the terminal-chunk follow-up: a masking error now redacts the chunk in place (empty content, original finish_reason and indices preserved) instead of dropping it, so the completion signal is never lost while still failing closed. Added |
83ec037 to
a95ed2b
Compare
|
Fixed the boundary-at-EOF case: a sentence terminator at the end of a chunk is no longer flushed until the following whitespace arrives, so an email split across the chunk seam ( |
a95ed2b to
5da41be
Compare
|
Reworked the output-masking path to stream incrementally while staying leak-safe, addressing the latest findings. Instead of trusting a sentence boundary (which can fall inside a name with an initial or an address across a newline), the path emits a prefix only when masking it in isolation matches the prefix of masking the whole buffer, with a lookahead margin still buffered past the cut; a straddling entity makes those differ and is held until complete. Buffering is bounded by a cap that masks a runaway punctuation-free run as one unit, and a masking error redacts only the affected chunk (or drops only the held tail) while the stream continues. Verified on a live proxy: the apply_to_output path now streams progressively (five chunks) with name, email and city all masked. |
5da41be to
32d15d4
Compare
|
Addressed the cap finding: the forced flush at the buffer cap now cuts a margin back from the end under the same stability check, so a PII value split exactly at the cap stays buffered and is masked whole rather than analyzed in halves. The other two new comments are a no-op interleaving (parsed chat chunks and raw Anthropic byte chunks never mix mid-token) and a passing assertion (the held placeholder prefix unmasks via the truncated-token fallback); details in the inline replies. |
|
All three are solid: Cap fix (buffer bypass): The stability-check cut with a trailing margin is the right approach. Emitting Carry split tokens (bytes interleaving): Already acknowledged in the thread — the Anthropic-native byte path and the parsed Impossible assertion ( All open findings resolved. The incremental streaming approach is sound, PII safety invariants are maintained across the boundary cases, and the regression tests pin the guarantees. This looks good to merge. |
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Masking error drops tool calls
- Streaming masking errors no longer clear accumulated tool/function state globally, and terminal tool/function buffers are only popped after successful rebuild.
- ✅ Fixed: Buffer cap not enforced
- The stream masking buffer now bounds held state even when stability checks fail by retaining only the trailing safety margin after forced cap handling.
- ✅ Fixed: Swallows intentional guardrail blocks
- Intentional guardrail interventions are re-raised from chunk, tail, and outer streaming masking error handlers instead of being redacted and swallowed.
You can send follow-ups to the cloud agent here.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 32d15d4. Configure here.
4eba8ab to
ad59bfe
Compare
|
Rebased onto the current Both output paths are retained on purpose. The default |
|
@greptileai please re-review the latest commit ( |
|
CI note: the three red checks are flaky external-API e2e jobs, not this diff. `ci/circleci: proxy_logging_guardrails_model_info_tests`, `ci/circleci: proxy_multi_instance_tests`, and `ci/circleci: proxy_pass_through_endpoint_tests` are all green on the `litellm_internal_staging` base commit and on several other open PRs at the same time. Their proxy configs (e.g. `otel_test_config.yaml`) load only bedrock and custom guardrails, no Presidio, so this change cannot reach them; the proxy also boots cleanly with Presidio active. Every job that does exercise this change is green: `lint`, `ci/circleci: guardrails_testing`, `guardrails-hooks / Run tests` (the mapped unit tests for this file), and all of the unit-test shards. The three failing jobs hit real OpenAI/Bedrock/Cohere/Datadog/GCS endpoints, which is the usual flaky surface, and a Rerun failed should clear them |
…he whole response The Presidio streaming post-call hooks (_stream_apply_output_masking for apply_to_output and _stream_pii_unmasking for output_parse_pii) collected every upstream chunk, reassembled the full completion with stream_chunk_builder at end-of-stream, ran Presidio over it, then emitted one reconstructed SSE chunk. Time-to-first-token collapsed to the total generation time and token-by-token streaming was lost whenever Presidio output handling was enabled. With the default presidio_filter_scope both, an apply_to_output masking instance is always created, so even the unmask configuration buffered the stream. Both paths now transform and forward chunks as they arrive. The unmask path replaces placeholder tokens per chunk, holding back only the trailing run that could still grow into a token so a placeholder split across SSE chunks (<PER + SON_1>) is still rewritten atomically. The mask path emits a prefix only when masking it in isolation matches the corresponding prefix of masking the whole buffer, with a lookahead margin still buffered past the cut, so an entity straddling the cut is detected and held until complete; past _PRESIDIO_STREAM_MAX_BUFFER the run is bounded without splitting an entity. Tool-call and legacy function-call argument fragments are accumulated per choice and transformed once the choice closes, content is buffered independently per choice index for correct n>1 streaming, raw Anthropic SSE bytes and /v1/responses events pass through with any held content flushed first so events never reorder, and a masking error redacts only the affected chunk (fail closed, keeping finish_reason) while the stream continues. Resolves LIT-3222
ad59bfe to
d4ed88c
Compare
Merging this PR will not alter performance
Comparing |
…he whole response (BerriAI#31503) The Presidio streaming post-call hooks (_stream_apply_output_masking for apply_to_output and _stream_pii_unmasking for output_parse_pii) collected every upstream chunk, reassembled the full completion with stream_chunk_builder at end-of-stream, ran Presidio over it, then emitted one reconstructed SSE chunk. Time-to-first-token collapsed to the total generation time and token-by-token streaming was lost whenever Presidio output handling was enabled. With the default presidio_filter_scope both, an apply_to_output masking instance is always created, so even the unmask configuration buffered the stream. Both paths now transform and forward chunks as they arrive. The unmask path replaces placeholder tokens per chunk, holding back only the trailing run that could still grow into a token so a placeholder split across SSE chunks (<PER + SON_1>) is still rewritten atomically. The mask path emits a prefix only when masking it in isolation matches the corresponding prefix of masking the whole buffer, with a lookahead margin still buffered past the cut, so an entity straddling the cut is detected and held until complete; past _PRESIDIO_STREAM_MAX_BUFFER the run is bounded without splitting an entity. Tool-call and legacy function-call argument fragments are accumulated per choice and transformed once the choice closes, content is buffered independently per choice index for correct n>1 streaming, raw Anthropic SSE bytes and /v1/responses events pass through with any held content flushed first so events never reorder, and a masking error redacts only the affected chunk (fail closed, keeping finish_reason) while the stream continues. Resolves LIT-3222

Relevant issues
Resolves LIT-3222
Linear ticket
LIT-3222
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
With Presidio output handling enabled, the streaming post-call hooks buffered the entire upstream stream, ran Presidio over the reassembled completion, then emitted one reconstructed SSE chunk. Time-to-first-token collapsed to the full generation time and token-by-token streaming was lost. Note that the default
presidio_filter_scope: bothalways instantiates anapply_to_outputmasking callback, so even anoutput_parse_piiconfiguration buffered the stream through that path. Both output paths now transform and forward chunks as they arrive.Repro is a live proxy on real
gpt-4o-ministreaming with the official Presidio analyzer + anonymizer docker images, swapping onlypresidio.pybetween the buffered and incremental versions. The client records the wall-clock arrival of every SSE chunk that carriesdelta.content. The prompt asks the model to generate a name, email, and city so theapply_to_outputmasking path (the one that was buffering under the default scope) is exercised end to end.Content arrives progressively after the fix instead of in a single delta at end-of-stream, while the model-generated name and email stay masked (
<PERSON>,<EMAIL_ADDRESS>) with no raw PII forwarded. A guardrail-free request on the same proxy streams 22 progressive chunks, confirming the upstream model streams normally and the buffering was entirely in the Presidio hooks.To reproduce locally
Type
🐛 Bug Fix
Changes
_stream_apply_output_masking(apply_to_output) and_stream_pii_unmasking(output_parse_pii) inlitellm/proxy/guardrails/guardrail_hooks/presidio.pyno longer callstream_chunk_builderto reassemble the whole completion. They share a small incremental rewriter that mutates each chat chunk in place:<PER+SON_1>) is still rewritten atomically_PRESIDIO_STREAM_MARGINcharacters of lookahead still buffered past the cut; any entity straddling the cut makes the two maskings differ, so the cut is held until the entity completes and is masked as one unit. Past_PRESIDIO_STREAM_MAX_BUFFERwith no safe cut the run is bounded without splitting an entityn>1streams stay correct/v1/responsesevents pass through untouched; any buffered masked or held content is flushed before such an event so a client never observes a later event ahead of earlier transformed textfinish_reason) and the stream keeps flowing to termination rather than truncating the whole response; intentional guardrail interventions (BlockedPiiEntityError,GuardrailRaisedException) still propagateTests extend the mapped file
tests/test_litellm/proxy/guardrails/guardrail_hooks/test_presidio.pywith regressions for incremental emission, cross-chunk token splits, per-choice independence, tool-call argument masking/unmasking, the no-split guarantee for an entity straddling a boundary, the buffer cap masking a runaway run without splitting an entity, flush-before-bytes ordering, terminal-chunk and tail masking-error survival (finish_reason preserved, fail closed), and guardrail-intervention propagation. Each fails on the unhardened implementation.