Follow-up to #131 (raised in review) — the Messages loop currently operates on a raw serde_json::Value for lossless upstream forwarding, and separately parses MessagesRequest for typed field access. This proposes a single per-request context holding both, so the loop gets safe typed access without a second parse and without giving up lossless forwarding.
Why raw Value is used today
The loop is a pass-through: it forwards the client's request to vLLM /v1/messages verbatim and only reads tools + rewrites messages/stream. Unlike RequestPayload::to_upstream_request (which transforms — flattens Codex namespaces, normalizes tool types, resolves tool_choice), there's no transform step here, so a typed round-trip would be pure overhead. One real catch: ContentBlock has #[serde(other)] Unknown, so deserializing→reserializing through MessagesRequest would silently drop unmodeled block types (redacted_thinking, future blocks, provider extensions) — raw Value forwards those untouched.
Proposed
A MessagesRequestContext { typed: MessagesRequest, raw: Value } built once per request:
- typed view for safe field access (tools, stream, model) in routing + the loop (today the handler parses
MessagesRequest only for routing and drops it; the loop mutates the raw Value alone);
- raw
Value remains the forwarded-upstream body (lossless);
- the two views only diverge where the loop touches the raw body — forcing
stream:false and appending the assistant + tool_result round to messages (append_round_to_history). The context would own those mutations so the views can't drift.
Alternatively, evaluate a to_upstream-style method on MessagesRequest that re-serializes losslessly — but note the loss is at the ContentBlock level (#[serde(other)] Unknown is a unit variant that captures no data), not top-level, so the top-level extra flatten doesn't cover it. This needs a block-level capture variant (or custom (de)serialize) on ContentBlock, and serde won't combine #[serde(other)] with a data-capturing catch-all on an internally-tagged enum — so it's not a drop-in. Confirm redacted_thinking/unknown blocks round-trip before adopting.
Scoped as a follow-up so it doesn't reshape the routing/loop plumbing inside #131. Ref: #131 review thread on messages_loop.rs.
Follow-up to #131 (raised in review) — the Messages loop currently operates on a raw
serde_json::Valuefor lossless upstream forwarding, and separately parsesMessagesRequestfor typed field access. This proposes a single per-request context holding both, so the loop gets safe typed access without a second parse and without giving up lossless forwarding.Why raw
Valueis used todayThe loop is a pass-through: it forwards the client's request to vLLM
/v1/messagesverbatim and only readstools+ rewritesmessages/stream. UnlikeRequestPayload::to_upstream_request(which transforms — flattens Codex namespaces, normalizes tool types, resolvestool_choice), there's no transform step here, so a typed round-trip would be pure overhead. One real catch:ContentBlockhas#[serde(other)] Unknown, so deserializing→reserializing throughMessagesRequestwould silently drop unmodeled block types (redacted_thinking, future blocks, provider extensions) — rawValueforwards those untouched.Proposed
A
MessagesRequestContext { typed: MessagesRequest, raw: Value }built once per request:MessagesRequestonly for routing and drops it; the loop mutates the rawValuealone);Valueremains the forwarded-upstream body (lossless);stream:falseand appending the assistant +tool_resultround tomessages(append_round_to_history). The context would own those mutations so the views can't drift.Alternatively, evaluate a
to_upstream-style method onMessagesRequestthat re-serializes losslessly — but note the loss is at theContentBlocklevel (#[serde(other)] Unknownis a unit variant that captures no data), not top-level, so the top-levelextraflatten doesn't cover it. This needs a block-level capture variant (or custom (de)serialize) onContentBlock, and serde won't combine#[serde(other)]with a data-capturing catch-all on an internally-tagged enum — so it's not a drop-in. Confirmredacted_thinking/unknown blocks round-trip before adopting.Scoped as a follow-up so it doesn't reshape the routing/loop plumbing inside #131. Ref: #131 review thread on
messages_loop.rs.