From a2dcb077be82d16fc481985c2a7c7c78fbde0372 Mon Sep 17 00:00:00 2001 From: Simo Lin Date: Mon, 20 Apr 2026 20:29:35 -0700 Subject: [PATCH 1/3] feat(protocols): implement P2 top-level fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What: - Extend `openai_protocol::common::StreamOptions` with an optional `include_obfuscation: Option` and derive `Default` so the struct is now the shared Chat-and-Responses stream-options type. - Introduce `PromptCacheRetention` (`"in-memory"` | `"24h"`), `ContextManagementEntry { type, compact_threshold }`, `ContextManagementType::Compaction`, and a `ConversationRef` untagged union (`string | { id: string }`) with an `as_id()` helper in `openai_protocol::common` for downstream tasks (P6 consumes `ConversationRef`). - Add six new top-level fields to `ResponsesRequest`: `prompt: Option`, `prompt_cache_key: Option`, `prompt_cache_retention: Option`, `safety_identifier: Option`, `stream_options: Option`, `context_management: Option>`, all `#[serde(skip_serializing_if = "Option::is_none")]`. Extend the `Default` impl accordingly. - Thread the six new fields through the gRPC regular-mode tool-loop continuation (`build_next_request` in `model_gateway/src/routers/grpc/regular/responses/common.rs`) so multi-turn loops preserve prompt template, cache key, safety identifier, streaming options, and context-management config. - Update `responses_to_chat` in `model_gateway/src/routers/grpc/regular/responses/conversions.rs` and two chat-completion spec tests to use `..StreamOptions::default()` (now that the struct has a `Default` impl) instead of field-exhaustive literals. - Add three serde round-trip tests in `crates/protocols/src/responses.rs` covering the full P2 field set, the `"in-memory"` retention variant, and absent-field omission on the wire. Why: - The SMG protocol types were missing six top-level fields required by the OpenAI Responses API spec (§Body Parameters: `prompt`, `prompt_cache_key`, `prompt_cache_retention`, `safety_identifier`, `stream_options`, `context_management`). Today spec-valid requests carrying any of these are silently dropped during deserialization, which masks client intent and breaks upstream routing decisions (cache reuse, obfuscation, compaction thresholds) that depend on these knobs. Adding them as typed `Option<_>` fields makes the gateway forward them losslessly and gives future tasks (P6, routing wiring) a typed surface to consume. How: - Grouped the new Responses-only types in `common.rs` next to the existing `ResponsePrompt` / `PromptVariable` cluster so every Responses-API-shared type lives in one place. - Reused the existing `common::StreamOptions` rather than introducing a Responses-specific duplicate: the OpenAI wire shape is identical modulo two optional fields, and collapsing to one type avoids drift between chat and responses streaming-options handling. `Default` derive lets existing call sites keep their explicit `include_usage` literal via `..StreamOptions::default()`. - Defined `ConversationRef` now (explicitly listed in the P2 audit entry) even though no existing field uses it yet, so P6 can migrate `ResponsesRequest::conversation` to `Option` without reopening this PR; kept `ResponsesRequest::conversation` as `Option` here to respect P6's dependency contract (P6 depends on P2 for the type; P6 owns the validator and `history.rs` persistence migration). - Chose `ContextManagementType` as a closed enum (only `Compaction`) rather than `String`: the spec today enumerates exactly one value, and a closed enum aligns with P5's fail-fast-on-unknown direction (unknown values serde-fail instead of being silently accepted). - All `Option<_>` fields use `skip_serializing_if = "Option::is_none"` so absent fields stay absent on the wire — verified by `test_responses_request_new_fields_omitted_when_absent`. Refs: P2 Signed-off-by: Simo Lin --- crates/protocols/src/common.rs | 69 ++++++- crates/protocols/src/responses.rs | 180 +++++++++++++++++- .../routers/grpc/regular/responses/common.rs | 10 + .../grpc/regular/responses/conversions.rs | 1 + model_gateway/tests/api/responses_api_test.rs | 78 ++++++++ model_gateway/tests/spec/chat_completion.rs | 2 + 6 files changed, 337 insertions(+), 3 deletions(-) diff --git a/crates/protocols/src/common.rs b/crates/protocols/src/common.rs index 3a6054194b..be753d6bd8 100644 --- a/crates/protocols/src/common.rs +++ b/crates/protocols/src/common.rs @@ -234,10 +234,16 @@ pub struct JsonSchemaFormat { // Streaming // ============================================================================ -#[derive(Debug, Clone, Deserialize, Serialize, schemars::JsonSchema)] +#[derive(Debug, Clone, Default, Deserialize, Serialize, schemars::JsonSchema)] pub struct StreamOptions { + /// Chat Completions / Completions: include usage block at end of stream. #[serde(skip_serializing_if = "Option::is_none")] pub include_usage: Option, + + /// Responses API: add random chars on `obfuscation` field of delta events + /// to normalize payload sizes. Defaults to `true` upstream when absent. + #[serde(skip_serializing_if = "Option::is_none")] + pub include_obfuscation: Option, } #[serde_with::skip_serializing_none] @@ -717,6 +723,67 @@ pub enum Detail { Auto, } +// ============================================================================ +// Responses API: prompt-cache retention & context management +// ============================================================================ + +/// Retention policy for prompt-cache entries on the Responses API. +/// +/// Spec: `prompt_cache_retention: "in-memory" | "24h"`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +pub enum PromptCacheRetention { + #[serde(rename = "in-memory")] + InMemory, + #[serde(rename = "24h")] + Duration24h, +} + +/// A single entry in the Responses API `context_management` array. +/// +/// Spec: each entry has `type` (currently only `"compaction"`) and an optional +/// `compact_threshold` token count. +#[serde_with::skip_serializing_none] +#[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] +pub struct ContextManagementEntry { + #[serde(rename = "type")] + pub r#type: ContextManagementType, + pub compact_threshold: Option, +} + +/// Type tag for [`ContextManagementEntry`]. Currently only `compaction` is +/// defined by the spec; the enum is kept small so unknown values serde-fail +/// (consistent with P5's fail-fast direction). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum ContextManagementType { + Compaction, +} + +// ============================================================================ +// Responses API: conversation reference +// ============================================================================ + +/// Reference to a conversation the response belongs to. +/// +/// Spec: `conversation: string | { id: string }`. Variant order matters for +/// `#[serde(untagged)]`: a bare JSON string succeeds as `Id`; an object falls +/// through to `Object`. +#[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] +#[serde(untagged)] +pub enum ConversationRef { + Id(String), + Object { id: String }, +} + +impl ConversationRef { + /// Return the underlying conversation id regardless of the wire shape. + pub fn as_id(&self) -> &str { + match self { + Self::Id(id) | Self::Object { id } => id.as_str(), + } + } +} + #[cfg(test)] mod tests { use serde::Deserialize; diff --git a/crates/protocols/src/responses.rs b/crates/protocols/src/responses.rs index 5f1664f416..66fa38ba03 100644 --- a/crates/protocols/src/responses.rs +++ b/crates/protocols/src/responses.rs @@ -9,8 +9,9 @@ use validator::{Validate, ValidationError}; use super::{ common::{ - default_true, validate_stop, ChatLogProbs, Function, GenerationRequest, - PromptTokenUsageInfo, StringOrArray, ToolChoice, ToolChoiceValue, ToolReference, UsageInfo, + default_true, validate_stop, ChatLogProbs, ContextManagementEntry, Function, + GenerationRequest, PromptCacheRetention, PromptTokenUsageInfo, ResponsePrompt, + StreamOptions, StringOrArray, ToolChoice, ToolChoiceValue, ToolReference, UsageInfo, }, sampling_params::{validate_top_k_value, validate_top_p_value}, }; @@ -931,6 +932,38 @@ pub struct ResponsesRequest { #[validate(custom(function = "validate_stop"))] pub stop: Option, + /// Reference to a prompt template and its variables. + /// Spec: body param `prompt` (ResponsePrompt). + #[serde(skip_serializing_if = "Option::is_none")] + pub prompt: Option, + + /// Stable cache key used by upstream to share prompt-prefix caches across + /// requests. Spec: body param `prompt_cache_key` (replaces `user`). + #[serde(skip_serializing_if = "Option::is_none")] + pub prompt_cache_key: Option, + + /// Retention policy for prompt-cache entries. + /// Spec: body param `prompt_cache_retention` (`"in-memory"` | `"24h"`). + #[serde(skip_serializing_if = "Option::is_none")] + pub prompt_cache_retention: Option, + + /// Stable user identifier for policy/abuse detection (max 64 chars on the + /// spec, but we do not enforce length here — routers may pass through). + /// Spec: body param `safety_identifier` (replaces `user` on request side). + #[serde(skip_serializing_if = "Option::is_none")] + pub safety_identifier: Option, + + /// Streaming-only options. Spec: body param `stream_options`. + /// On the Responses API the only documented field is `include_obfuscation`. + #[serde(skip_serializing_if = "Option::is_none")] + pub stream_options: Option, + + /// Per-request context-management configuration. + /// Spec: body param `context_management` — array of entries describing how + /// the upstream should compact context for this request. + #[serde(skip_serializing_if = "Option::is_none")] + pub context_management: Option>, + /// Top-k sampling parameter (SGLang extension) #[serde(default = "default_top_k")] #[validate(custom(function = "validate_top_k_value"))] @@ -985,6 +1018,12 @@ impl Default for ResponsesRequest { frequency_penalty: None, presence_penalty: None, stop: None, + prompt: None, + prompt_cache_key: None, + prompt_cache_retention: None, + safety_identifier: None, + stream_options: None, + context_management: None, top_k: default_top_k(), min_p: 0.0, repetition_penalty: default_repetition_penalty(), @@ -1966,4 +2005,141 @@ mod tests { let serialized = serde_json::to_value(&tool).expect("file_search tool should serialize"); assert_eq!(serialized, payload); } + + // ------------------------------------------------------------------ + // P2: new top-level ResponsesRequest fields + // ------------------------------------------------------------------ + + /// Acceptance: the six new top-level fields deserialize and re-serialize + /// without loss (`prompt`, `prompt_cache_key`, `prompt_cache_retention`, + /// `safety_identifier`, `stream_options`, `context_management`). + #[test] + fn test_responses_request_new_top_level_fields_round_trip() { + let payload = json!({ + "model": "gpt-5.4", + "input": "hello", + "prompt": { + "id": "pmpt_abc", + "variables": { + "name": "ada", + "picture": { + "type": "input_image", + "image_url": "https://example.com/pic.png", + "detail": "high" + } + }, + "version": "1" + }, + "prompt_cache_key": "pck-123", + "prompt_cache_retention": "24h", + "safety_identifier": "sid-123", + "stream_options": { "include_obfuscation": false }, + "context_management": [{ + "type": "compaction", + "compact_threshold": 4096 + }] + }); + + let request: ResponsesRequest = + serde_json::from_value(payload.clone()).expect("request should deserialize"); + + assert!(request.prompt.is_some()); + assert_eq!( + request.prompt.as_ref().map(|p| p.id.as_str()), + Some("pmpt_abc") + ); + assert_eq!( + request.prompt.as_ref().and_then(|p| p.version.as_deref()), + Some("1") + ); + assert_eq!(request.prompt_cache_key.as_deref(), Some("pck-123")); + assert_eq!( + request.prompt_cache_retention, + Some(PromptCacheRetention::Duration24h) + ); + assert_eq!(request.safety_identifier.as_deref(), Some("sid-123")); + assert_eq!( + request + .stream_options + .as_ref() + .and_then(|s| s.include_obfuscation), + Some(false) + ); + let ctx = request + .context_management + .as_ref() + .expect("context_management must round-trip"); + assert_eq!(ctx.len(), 1); + assert_eq!( + ctx[0].r#type, + crate::common::ContextManagementType::Compaction + ); + assert_eq!(ctx[0].compact_threshold, Some(4096)); + + // Re-serialize and confirm the wire form matches the inputs. + let reserialized = serde_json::to_value(&request).expect("should serialize"); + assert_eq!(reserialized["prompt"]["id"], "pmpt_abc"); + assert_eq!(reserialized["prompt_cache_key"], "pck-123"); + assert_eq!(reserialized["prompt_cache_retention"], "24h"); + assert_eq!(reserialized["safety_identifier"], "sid-123"); + assert_eq!(reserialized["stream_options"]["include_obfuscation"], false); + assert_eq!(reserialized["context_management"][0]["type"], "compaction"); + assert_eq!( + reserialized["context_management"][0]["compact_threshold"], + 4096 + ); + } + + /// `prompt_cache_retention` accepts the other spec value and serializes + /// back with the hyphenated rename. + #[test] + fn test_prompt_cache_retention_in_memory_round_trip() { + let request: ResponsesRequest = serde_json::from_value(json!({ + "model": "gpt-5.4", + "input": "hello", + "prompt_cache_retention": "in-memory" + })) + .expect("should deserialize"); + + assert_eq!( + request.prompt_cache_retention, + Some(PromptCacheRetention::InMemory) + ); + + let reserialized = serde_json::to_value(&request).expect("should serialize"); + assert_eq!(reserialized["prompt_cache_retention"], "in-memory"); + } + + /// Absent fields must stay absent on the wire (no `"prompt": null` etc.), + /// matching every other `Option<_>` field on `ResponsesRequest`. + #[test] + fn test_responses_request_new_fields_omitted_when_absent() { + let request: ResponsesRequest = serde_json::from_value(json!({ + "model": "gpt-5.4", + "input": "hello" + })) + .expect("should deserialize"); + + assert!(request.prompt.is_none()); + assert!(request.prompt_cache_key.is_none()); + assert!(request.prompt_cache_retention.is_none()); + assert!(request.safety_identifier.is_none()); + assert!(request.stream_options.is_none()); + assert!(request.context_management.is_none()); + + let serialized = serde_json::to_value(&request).expect("should serialize"); + for key in [ + "prompt", + "prompt_cache_key", + "prompt_cache_retention", + "safety_identifier", + "stream_options", + "context_management", + ] { + assert!( + serialized.get(key).is_none(), + "field {key} should be skipped when absent" + ); + } + } } diff --git a/model_gateway/src/routers/grpc/regular/responses/common.rs b/model_gateway/src/routers/grpc/regular/responses/common.rs index af6a3b1558..426e991158 100644 --- a/model_gateway/src/routers/grpc/regular/responses/common.rs +++ b/model_gateway/src/routers/grpc/regular/responses/common.rs @@ -406,6 +406,16 @@ pub(super) fn build_next_request( frequency_penalty: current_request.frequency_penalty, presence_penalty: current_request.presence_penalty, stop: current_request.stop, + // Responses API top-level fields (P2): propagate per-request knobs so + // multi-turn tool-loop continuations keep the same prompt template, + // cache key, safety identifier, streaming options, and context- + // management config as the original request. + prompt: current_request.prompt, + prompt_cache_key: current_request.prompt_cache_key, + prompt_cache_retention: current_request.prompt_cache_retention, + safety_identifier: current_request.safety_identifier, + stream_options: current_request.stream_options, + context_management: current_request.context_management, top_k: current_request.top_k, min_p: current_request.min_p, repetition_penalty: current_request.repetition_penalty, diff --git a/model_gateway/src/routers/grpc/regular/responses/conversions.rs b/model_gateway/src/routers/grpc/regular/responses/conversions.rs index 7ba5c446a2..7393652162 100644 --- a/model_gateway/src/routers/grpc/regular/responses/conversions.rs +++ b/model_gateway/src/routers/grpc/regular/responses/conversions.rs @@ -187,6 +187,7 @@ pub(crate) fn responses_to_chat(req: &ResponsesRequest) -> Result Date: Mon, 20 Apr 2026 21:01:40 -0700 Subject: [PATCH 2/3] refactor(protocols): drop speculative ConversationRef from P2 (cycle 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tech Lead P2 cycle-1 REJECT: ConversationRef was `pub` with zero callers — §7 "Every new pub item must be imported somewhere". P6 owns the ConversationRef type AND its migration into ResponsesRequest::conversation; pre-landing it here is speculative. Removed the enum + its impl + section comment. No change to the 6 new top-level ResponsesRequest fields or other P2 additions. Refs: P2 Signed-off-by: Simo Lin --- crates/protocols/src/common.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/crates/protocols/src/common.rs b/crates/protocols/src/common.rs index be753d6bd8..0d6a3ddac2 100644 --- a/crates/protocols/src/common.rs +++ b/crates/protocols/src/common.rs @@ -759,31 +759,6 @@ pub enum ContextManagementType { Compaction, } -// ============================================================================ -// Responses API: conversation reference -// ============================================================================ - -/// Reference to a conversation the response belongs to. -/// -/// Spec: `conversation: string | { id: string }`. Variant order matters for -/// `#[serde(untagged)]`: a bare JSON string succeeds as `Id`; an object falls -/// through to `Object`. -#[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(untagged)] -pub enum ConversationRef { - Id(String), - Object { id: String }, -} - -impl ConversationRef { - /// Return the underlying conversation id regardless of the wire shape. - pub fn as_id(&self) -> &str { - match self { - Self::Id(id) | Self::Object { id } => id.as_str(), - } - } -} - #[cfg(test)] mod tests { use serde::Deserialize; From 4e58478e99b2fef426fc9869a058237dc38f621c Mon Sep 17 00:00:00 2001 From: Simo Lin Date: Tue, 21 Apr 2026 06:49:46 -0700 Subject: [PATCH 3/3] fix(gateway): preserve caller stream_options in responses_to_chat (P2 bot feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What ---- - model_gateway/src/routers/grpc/regular/responses/conversions.rs: - `responses_to_chat` no longer overwrites `ResponsesRequest.stream_options` with a fresh `StreamOptions { include_usage: Some(true), ..default() }` literal. The caller's `stream_options` is now cloned forward and only `include_usage` is defaulted to `Some(true)` when the caller did not set it. Non-streaming requests still intentionally drop `stream_options`. - Drop the now test-only `StreamOptions` import from the module-level use block and re-import it inside `mod tests` so the non-test build does not emit `unused_import`. - Add three regression tests: * `test_stream_options_include_obfuscation_roundtrip` — proves `include_obfuscation: Some(false)` survives the conversion and `include_usage` still defaults to `Some(true)` when unset. * `test_stream_options_caller_include_usage_preserved` — proves the caller's `include_usage: Some(false)` is not clobbered. * `test_stream_options_non_streaming_dropped` — confirms `stream=false` yields `stream_options: None` even when the caller set fields. Why --- Codex bot (P1 Major) on PR #1278 flagged that a streaming request with `stream_options.include_obfuscation=false` was being converted to a ChatCompletionRequest whose `include_obfuscation` silently became `None`, discarding caller intent. This is a wire-semantics bug introduced by the P2 schema additions: adding `stream_options` to `ResponsesRequest` without threading it through the gRPC conversion path left the field accepted but ignored. How --- Switch the literal construction to `let mut opts = req.stream_options.clone().unwrap_or_default();` followed by an `if opts.include_usage.is_none()` backfill. This preserves every caller- supplied field (including future `StreamOptions` additions via `Default`) while keeping the existing `include_usage=true` behaviour internal pipeline consumers rely on for end-of-stream usage emission. Scope ----- Bug-fix only; no protocol schema changes, no new CLI flags, no bindings updates. Cross-parameter validation (rejecting `stream_options` with `stream=false`) is deliberately out of P2's schema-only charter and is being tracked as a follow-up on the codex P2 dismissal. Verify ------ - `cargo check --workspace --tests` → clean - `cargo test -p smg --lib` → 559 passed (3 new) - `cargo test -p openai-protocol` → 19 passed - `cargo clippy -p smg -p openai-protocol --lib --bins --tests -- -D warnings` → clean - `cargo +nightly fmt --all -- --check` → clean Refs: #1278 Signed-off-by: Simo Lin --- .../grpc/regular/responses/conversions.rs | 80 +++++++++++++++++-- 1 file changed, 73 insertions(+), 7 deletions(-) diff --git a/model_gateway/src/routers/grpc/regular/responses/conversions.rs b/model_gateway/src/routers/grpc/regular/responses/conversions.rs index 7393652162..0bfa7d4db8 100644 --- a/model_gateway/src/routers/grpc/regular/responses/conversions.rs +++ b/model_gateway/src/routers/grpc/regular/responses/conversions.rs @@ -9,9 +9,7 @@ use openai_protocol::{ chat::{ChatCompletionRequest, ChatCompletionResponse, ChatMessage, MessageContent}, - common::{ - FunctionCallResponse, JsonSchemaFormat, ResponseFormat, StreamOptions, ToolCall, UsageInfo, - }, + common::{FunctionCallResponse, JsonSchemaFormat, ResponseFormat, ToolCall, UsageInfo}, responses::{ ResponseContentPart, ResponseInput, ResponseInputOutputItem, ResponseOutputItem, ResponseReasoningContent::ReasoningText, ResponseStatus, ResponsesRequest, @@ -184,11 +182,15 @@ pub(crate) fn responses_to_chat(req: &ResponsesRequest) -> Result