feat(rehydrate): restore previous_response_id in streaming Responses - #1004
Merged
leseb merged 5 commits intoSep 9, 2026
Merged
Conversation
Extend the non-streaming previous_response_id restore (praxis-proxy#962) to the streaming (SSE) path. When history was rehydrated, the proxy strips previous_response_id from the upstream request, so the backend echoes null in every response-lifecycle frame. The rehydrate filter now rewrites those frames incrementally as they arrive, restoring the caller's id, without ever buffering the stream. Frames are spliced, not reconstructed: only a lifecycle frame's data payload is rewritten in place, so id:, retry:, comments, and unknown SSE fields survive byte-for-byte. At most one partial frame is buffered across chunk boundaries, and the boundary scan resumes where it left off so a frame split across many chunks is scanned once, never re-scanned from byte zero. Streaming eligibility mirrors the non-streaming path: only a 200 OK, identity-coded, non-ranged text/event-stream from a rehydrated turn is rewritten, and any response carrying a body validator or integrity digest is declined up front via describes_exact_upstream_bytes. Any oversized frame or SSE parse error fails open and flushes the remainder raw, never erroring the request. Add unit tests, a full-flow integration test, a live vLLM SDK test, and a synthetic native-continuation-stream inference fixture. Refs praxis-proxy#932 Signed-off-by: Sébastien Han <seb@redhat.com>
leseb
enabled auto-merge
September 8, 2026 14:16
franciscojavierarceo
approved these changes
Sep 8, 2026
franciscojavierarceo
left a comment
franciscojavierarceo
left a comment
Contributor
There was a problem hiding this comment.
reviewed the current head; no actionable findings.
praxis-bot
reviewed
Sep 8, 2026
praxis-bot
left a comment
praxis-bot
left a comment
There was a problem hiding this comment.
Solid implementation. The streaming SSE previous_response_id restore is well-designed: bounded memory via max_buffer_bytes, fail-open semantics that never drop or corrupt bytes, zero-copy fast paths for pass-through frames, and linear scanning with cross-chunk resume state that avoids quadratic re-scanning. The WHATWG line terminator handling (LF/CR/CRLF with ambiguous trailing CR deferral) is correct, and the scan_complete_frames / accumulate_frame / finalize_view pipeline correctly handles lazy buffer materialization. Test coverage is thorough — chunk boundaries, overflow, bare CR terminators, zero-copy verification, and the carry-over compaction path are all exercised. One convention pattern to address.
Three inline comments in the streaming rehydrate tests narrated exactly what the following assertion already checks. Per the CONTRIBUTING guidance to prefer assertion messages over inline comments, fold the delta pass-through comment into its assertion message and drop the two that merely restated an existing assertion message. Substantive "why" comments (regression provenance, subtle SSE invariants, issue refs) are kept, as CONTRIBUTING permits. Signed-off-by: Sébastien Han <seb@redhat.com>
Follow praxis-bot's guidance to remove inline comments that merely restate what an assertion message or the test function name already conveys, and fold one genuinely-new-context comment (the end-of-stream guard rationale) into its assertion message. The substantive "why" comments documenting regression provenance, subtle SSE framing invariants, and issue references are kept as CONTRIBUTING permits. Signed-off-by: Sébastien Han <seb@redhat.com>
Bring the streaming previous_response_id restore branch (Closes praxis-proxy#932) up to date with main through praxis-proxy#1009. Conflicts resolved in the inference coverage fixtures: both sides added a new scenario (responses/native-continuation-stream on this branch, responses/chat-tool-echo on main), so the auto-merged scenario/recording totals were undercounted. Corrected the checked-in manifest assertion to the true combined totals features=23, scenarios=22, recordings=27, and regenerated tests/integration/fixtures/inference/README.md from the manifest. The vLLM SDK test merged cleanly (this branch's streaming test plus main's changes). Signed-off-by: Sébastien Han <seb@redhat.com>
Collaborator
Author
|
should stabilize after #1011 merges |
leseb
added a commit
to leseb/praxis-ai
that referenced
this pull request
Sep 9, 2026
Resolve test-only conflicts from praxis-proxy#1004 (streaming previous_response_id restore), praxis-proxy#1026 (audio/resource-link MCP results), and praxis-proxy#1012: - rehydrate/tests.rs: union the imports — keep origin's openai::sse {SseFrame, SseFrameParser} and this branch's PendingApprovalRecord. - test_openai_responses_vllm.py MCPHandler tools/call: keep the approval round-trip tool-call counter and adopt praxis-proxy#1026's get_weather_map resource_link (non-text) content branch. - mcp_dispatch/tests.rs: pass None for the approval_request_id argument praxis-proxy#1026's process_call_result test did not supply. Signed-off-by: Sébastien Han <seb@redhat.com>
leseb
added a commit
to leseb/praxis-ai
that referenced
this pull request
Sep 9, 2026
…#1026) into leseb/fix-batched-tool-calls Reconcile the latest main with the batched/parallel model tool-call support on this branch. - vLLM SDK test mock (test_openai_responses_vllm.py): serve get_weather, get_time (this branch's two-tool batch test) and get_weather_map (praxis-proxy#1026's non-text resource_link test) together, dispatching each in the tools/call handler so both scenarios run against one mock. - mcp_dispatch tests: pass this branch's max_result_bytes cap argument to content_blocks_to_output and process_call_result in praxis-proxy#1026's new audio/resource-link/complex-schema coverage. Signed-off-by: Sébastien Han <seb@redhat.com>
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
3 participants
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.
Summary
Completes issue #932 by extending the
previous_response_idrestore to the streaming (SSE) Responses path: when history was rehydrated the proxy stripsprevious_response_idfrom the upstream request, so the backend echoesnullin every response-lifecycle frame, and the rehydrate filter now rewrites those frames incrementally as they arrive — splicing only a lifecycle frame'sdata:JSON in place soid:,retry:, comments, and delta frames pass through byte-for-byte — without ever buffering the stream. At most one partial frame is held across chunk boundaries and the boundary scan resumes where it left off, so a frame split across many chunks is scanned once, and any oversized frame or SSE parse error fails open and flushes the remainder raw. The buffered half already shipped in #962, so this is the smallest change that finishes the issue.Related issue
Closes #932
Validation
cargo test -p praxis-ai-apis --lib -- openai::responses::rehydrate(100 passed)cargo test -p praxis-tests-integration --test suite -- full_flow_previous_response_id(3 passed);make test-inference-fixtures(all passed); new syntheticresponses/native-continuation-streamfixture and live vLLM SDK testtest_streaming_rehydrated_response_echoes_previous_response_idmake lint(green;check-inference ok: features=22 scenarios=21 recordings=26); alsomake buildandmake docgreenChecklist
Signed-off-bytrailer.Breaking changes
None.