Skip to content

feat: add per-request context to the /v1/messages loop (#137) - #249

Open
Zheng-Lu wants to merge 1 commit into
vllm-project:mainfrom
Zheng-Lu:feat/messages-request-context
Open

feat: add per-request context to the /v1/messages loop (#137)#249
Zheng-Lu wants to merge 1 commit into
vllm-project:mainfrom
Zheng-Lu:feat/messages-request-context

Conversation

@Zheng-Lu

@Zheng-Lu Zheng-Lu commented Sep 6, 2026

Copy link
Copy Markdown

Summary

Closes #137. Follow-up to #131, addressing the code review concern that the Messages loop took a bare serde_json::Value at a public API boundary while the handler separately parsed a MessagesRequest for typed field access.

Technical Details

Introduces MessagesRequestContext { typed: MessagesRequest, raw: Value }, built once per request, and threads it through run_messages_loop and run_messages_stream in place of the untyped body.

The raw body stays the thing forwarded upstream, and is deliberately not re-serialized from the typed view. ContentBlock catches unmodeled block types in #[serde(other)] Unknown and several variants model only the fields the gateway reads, so a typed round-trip would drop cache_control and is_error and collapse image/redacted_thinking into a literal {"type":"unknown"} block.

  • The typed view exposes only tools(), stream() and model() — the fields the loops read and never mutate. messages and system are unreachable through it, so a stale typed view cannot be read back after a round is appended.
  • The context owns every mutation to the upstream body (force_stream, append_round) plus the native web-search budget, replacing the two duplicated append_round_to_history copies with one method.
  • The two views diverge only where the gateway rewrites for upstream: normalize_native_web_search rewrites the native web_search_20250305 declaration in the raw body, while the typed view keeps the client's original so the tool seam can still classify it.
  • MessagesRequestContext::new reuses the handler's routing parse, so neither view is built twice and a proxied request never pays for a raw parse it does not use. Native web-search validation folds into the constructor, still ahead of a streaming response committing its status, and validate_native_web_search_request drops off the public API.

Also removes the single-field ResolvedCall/ResolvedStreamCall wrappers, which every caller immediately unwrapped, and a redundant deep clone of the assistant turn per round in the non-streaming loop.

Note this keeps routing on the full MessagesRequest parse, so a body that declares a gateway tool but fails that parse on an unrelated field still falls through to the proxy silently. That behaviour is unchanged here and is worth a separate issue.

Test Plan

  • cargo fmt -- --check, cargo clippy --all-targets -- -D warnings and cargo test --workspace all clean: 889 passed, 0 failed, cassette replays included.
  • New messages_loop_preserves_unmodeled_blocks_and_tool_result_fields_across_rounds covers image, redacted_thinking, citations and a tool_result carrying is_error and cache_control, asserting they reach upstream unchanged across a gateway round.
  • Negative control: pointing upstream_body at the typed view fails 8 tests, including the existing Claude Code cache_control replays, which confirms the guard is not vacuous.

Closes vllm-project#137. Follow-up to vllm-project#131, addressing the code review concern that
the Messages loop took a bare `serde_json::Value` at a public API
boundary while the handler separately parsed a `MessagesRequest` for
typed field access.

Introduces `MessagesRequestContext { typed: MessagesRequest, raw: Value }`,
built once per request, and threads it through `run_messages_loop` and
`run_messages_stream` in place of the untyped body.

The raw body stays the thing forwarded upstream, and is deliberately not
re-serialized from the typed view. `ContentBlock` catches unmodeled block
types in `#[serde(other)] Unknown` and several variants model only the
fields the gateway reads, so a typed round-trip would drop `cache_control`
and `is_error` and collapse `image`/`redacted_thinking` into a literal
`{"type":"unknown"}` block.

- The typed view exposes only `tools()`, `stream()` and `model()` — the
  fields the loops read and never mutate. `messages` and `system` are
  unreachable through it, so a stale typed view cannot be read back after
  a round is appended.
- The context owns every mutation to the upstream body (`force_stream`,
  `append_round`) plus the native web-search budget, replacing the two
  duplicated `append_round_to_history` copies with one method.
- The two views diverge only where the gateway rewrites for upstream:
  `normalize_native_web_search` rewrites the native `web_search_20250305`
  declaration in the raw body, while the typed view keeps the client's
  original so the tool seam can still classify it.
- `MessagesRequestContext::new` reuses the handler's routing parse, so
  neither view is built twice and a proxied request never pays for a raw
  parse it does not use. Native web-search validation folds into the
  constructor, still ahead of a streaming response committing its status,
  and `validate_native_web_search_request` drops off the public API.

Also removes the single-field `ResolvedCall`/`ResolvedStreamCall`
wrappers, which every caller immediately unwrapped, and a redundant deep
clone of the assistant turn per round in the non-streaming loop.

Note this keeps routing on the full `MessagesRequest` parse, so a body
that declares a gateway tool but fails that parse on an unrelated field
still falls through to the proxy silently. That behaviour is unchanged
here and is worth a separate issue.

Test Plan:
- `cargo fmt -- --check`, `cargo clippy --all-targets -- -D warnings` and
  `cargo test --workspace` all clean: 889 passed, 0 failed, cassette
  replays included.
- New `messages_loop_preserves_unmodeled_blocks_and_tool_result_fields_across_rounds`
  covers `image`, `redacted_thinking`, `citations` and a `tool_result`
  carrying `is_error` and `cache_control`, asserting they reach upstream
  unchanged across a gateway round.
- Negative control: pointing `upstream_body` at the typed view fails 8
  tests, including the existing Claude Code cache_control replays, which
  confirms the guard is not vacuous.

Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
@Zheng-Lu Zheng-Lu changed the title feat: add per-request context to the /v1/messages loop feat: add per-request context to the /v1/messages loop (#137) Sep 6, 2026

@franciscojavierarceo franciscojavierarceo 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.

the handler builds both views from the same request, and the loops keep forwarding the raw body when appending tool rounds. i didn't find a behavior regression in the refactor. this was a source review; i haven't independently run the tests.

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.

/v1/messages loop: per-request context (typed MessagesRequest + raw Value)

2 participants