Skip to content

fix: honor WebSocket response storage with bounded sessions - #257

Merged
franciscojavierarceo merged 8 commits into
vllm-project:mainfrom
LOGO127:feat/core-response-sessions
Sep 11, 2026
Merged

fix: honor WebSocket response storage with bounded sessions#257
franciscojavierarceo merged 8 commits into
vllm-project:mainfrom
LOGO127:feat/core-response-sessions

Conversation

@LOGO127

@LOGO127 LOGO127 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #238 by connecting the draft's typed core response sessions to the existing WebSocket multiplexer. A WebSocket response.create with store: false now retains continuation state in memory on that connection, with no durable response/item writes. Same-connection continuation and generate: false prewarm work with storage disabled; reconnecting requires replaying context for unstored responses.

  • Preserve the current AgentPipeline, concurrent named lanes, default-lane FIFO, request/event limits, tool execution, and stored-response continuation. Integrate main at 3b70271 while preserving the original contributor's commit history.
  • Retain one latest checkpoint per lane. Bound each connection to 128 lifetime lanes, 32,768 items and 16 MiB per checkpoint, and 32 MiB of aggregate serialized checkpoints. Shared/pinned parents and prepared replacements remain charged until released; reserve before durable writes. These are retention limits, not measured process-memory bounds.
  • Resolve parents when execution starts. Immediate admission 429 responses preserve existing checkpoints and accepted queued work. Execution and validly routed payload-validation failures evict only a matching same-lane parent; failed forks preserve their source. Disconnect cancels request tasks and waits for continuation leases to release before disposing connection state.
  • Preserve custom/shell call-output conversion on continuation. Canonicalize replayed compaction windows before retention accounting, pending-call checks, and commit call-ID validation.
  • A stored child of an unstored parent persists its complete canonical history. Explicit Conversations, no-session HTTP, and split-execution policies remain unchanged. RequestContext gains the draft's optional continuation field; downstream struct literals need to supply it.

Test Plan

Follow-up 9666615 integrates the latest main and supplies continuation: None in its new terminal-status test fixture. The original CI failure was reproduced on the merged tree. CI's exact cargo clippy --all-targets -- -D warnings now passes locally, as do 94 terminal-status, incomplete-response, WebSocket, and session-continuation tests and the changed-file pre-commit hooks.

Prior full-workspace validation on a856549, with all features enabled:

  • 1,325 passed, 9 ignored across workspace library tests, binary tests, all 42 integration-test targets, and doctests. Ran cargo test --locked --workspace --all-features by target (--lib, --bins, each --test, and --doc) to limit disk use, with debug symbols and incremental compilation disabled.
  • 49 WebSocket tests and 34 public core continuation tests pass. New regressions cover response/item row counts, default/named lanes, prewarm, disabled storage, reconnect rejection, stored-child promotion, custom tools, validation eviction, count/byte overload ordering, retention limits, pinned forks, disconnect cleanup, and replayed compaction.
  • Regression tests reproduced unwanted persistence, disabled-storage failure, missing validation eviction, and both compaction bugs before their fixes. Existing WebSocket custom-tool continuation also caught the integration regression before its correction.
  • cargo clippy --locked --workspace --all-targets --all-features -- -D warnings passes.
  • cargo fmt -- --check and all repository-wide pre-commit hooks pass.

Storage validation used SQLite and disabled storage with local mock/cassette upstreams. The nine skips are eight PostgreSQL tests and one doctest; no live-model validation was performed.

Signed-off-by: luozijian <luozijian0924@gmail.com>
Signed-off-by: luozijian <luozijian0924@gmail.com>
@LOGO127

LOGO127 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up 01ebcb7cbbccdcc9f399275f2b7c09c2b19d7e75 releases the ending lease's parent checkpoint reference before marking the session idle and notifying waiters, both on publication and disposal. Other holders still retain and charge shared checkpoints.

This is a two-file, core-only follow-up (+112 lines, including three deterministic wake-observer tests) directly on signed 51c3613. It does not add transport integration or incorporate the separate main-905d integration worktree.

The personally signed commit's two blobs match the previously validated isolated delivery tree exactly. That tree passed all three focused handoff tests, workspace/all-targets/all-features Clippy with warnings denied, fmt, and applicable changed-file hooks. The complete offline/locked workspace/all-features test rerun exited successfully; eight PostgreSQL tests and one doctest remain ignored.

For completeness: the first complete run failed in the existing oversized-body test with ConnectionReset while sending 11 MiB. Its isolated rerun and the second full run passed. I have not established baseline causality for that failure and am not treating the passing rerun as proof it cannot recur. No fresh GPU/live-model/full PostgreSQL verification is claimed.

AI assistance was used in implementation and validation. The contributor performed the local review/sign-off; publication preserves that commit without rewriting history. This remains a draft prerequisite, not completion of #238 or an assigned ownership role.

Signed-off-by: luozijian <luozijian0924@gmail.com>
@LOGO127

LOGO127 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this draft with personally signed merge d6f36de5baf2a2c5ac9607854719c723288f8be4, incorporating upstream 3bb55fdf55ea1a7e756cf79694b187909faf62b7 without rewriting prior signed commits.

The integration preserves upstream tool-search metadata and transient session semantics. Calls are retained once before their outputs; compacted session parents restore loaded-tool metadata from the retained checkpoint rather than querying nonexistent durable storage. A disabled-storage regression test covers the latter. Two upstream test context initializers now specify continuation: None; four split-execution test futures are boxed for the strict large-futures lint.

The signed tree matches the validated candidate exactly. All 28 session tests pass, as do offline/locked workspace/all-features tests, workspace/all-targets/all-features Clippy with warnings denied, and applicable outgoing-file hooks. Eight PostgreSQL tests and one doctest remain ignored. The initial merge failed four session tests with duplicate call IDs; these passed after reconciling history ownership. A Windows-mounted run also failed the cassette executable-mode assertion; the complete passing run used a Git archive of the same tree on native Linux, preserving tracked file modes without changing cassette content or assertions.

AI-assisted implementation and verification; contributor personally reviewed and signed. No GPU/live-model/PostgreSQL verification or complete WebSocket integration is claimed. This remains a draft core prerequisite.

franciscojavierarceo added a commit that referenced this pull request Sep 9, 2026
## Summary

Fixes #256.

- Preserve message `input_file` fields with `InputContent::InputFile`,
reusing the existing typed file shape.
- Reject unsupported message files before storage work and after
complete history rehydration. The public `upstream_request` preparation
step checks the same contract for composable callers.
- Reuse the existing 400 `invalid_request_error` mapping across typed
HTTP/streaming, WebSocket, local prewarm, and compaction paths. No
inference is sent for rejected input, including files restored from
durable history.
- Preserve eligible raw proxy request bytes, text/image content, and
structured function/custom tool-output files. Document the distinction
from Codex local file mentions/read tools.

No file upload endpoint, file-ID resolver, document parser, OCR, or new
dependency. The added public enum variant may require downstream
exhaustive matches to be updated.

This is an independent main-based fix. #257 touches the same rehydration
function; its eventual integration must place validation in the shared
`rehydrate_with_continuation` body, not only the no-session wrapper. A
separate local combined candidate validates that resolution; its
session-specific tests are not included in this standalone diff.

## Test Plan

WSL Ubuntu x86_64, Rust 1.98.0, locked offline dependencies, local mock
upstreams and isolated SQLite:

- Original main `74c6b2d` with the 30 contract tests: **24 failures / 6
passing controls**. Raw stored-history fixtures retain file JSON
independently of deserialization.
- Fixed candidate: **30/30 contract tests passed**.
- Full workspace/all features with `--no-fail-fast`: **909 passed / 9
ignored** across 42 top-level test/doctest targets, excluding duplicate
nested subprocess summaries.
- Workspace/all targets/all features Clippy with `-D warnings`, rustfmt,
applicable seven-path pre-commit hooks, and whitespace checks passed.

The first full run hit one existing 11 MB body-limit test's connection
reset. Its test and pre-deserialization handler code are unchanged; the
focused rerun and the complete rerun passed. That observation is
retained, not hidden or counted as a passing first run.

The ignored cases include eight PostgreSQL-specific tests and one
existing doctest. No PostgreSQL, live model/GPU, MSRV, Python
wheel/release or performance validation is claimed for this patch.

AI assistance was used for implementation and verification. The
contributor reviewed the patch and created the DCO-signed commit
`fab52ff`; the signed tree exactly matches the verified review snapshot.

Signed-off-by: luozijian <luozijian0924@gmail.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Co-authored-by: Francisco Javier Arceo <farceo@redhat.com>
LOGO127 and others added 3 commits September 10, 2026 05:26
Assisted-by: OpenAI Codex
Signed-off-by: luozijian <luozijian0924@gmail.com>
Assisted-by: OpenAI Codex
Signed-off-by: luozijian <luozijian0924@gmail.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
@franciscojavierarceo franciscojavierarceo changed the title feat: add bounded transient response sessions to core fix: honor WebSocket response storage with bounded sessions Sep 11, 2026
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
@franciscojavierarceo
franciscojavierarceo merged commit 2509342 into vllm-project:main Sep 11, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

store: false still persists and resolves previous_response_id across WebSocket connections (breaks ZDR expectation)

2 participants