From e89d2d30de9ed7f5a1fb35a7feb1b533970de683 Mon Sep 17 00:00:00 2001 From: Manan Patel <144215944+mananpatelll@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:54:08 -0700 Subject: [PATCH] fix(frontend): accept safety_identifier, prompt_cache_retention, verbosity in chat These are valid OpenAI Chat Completions fields that Dynamo doesn't act on, so clients sending them got a 400. Check their values and drop them, the way /v1/responses already handles them. Legacy completions still rejects them. Signed-off-by: Manan Patel <144215944+mananpatelll@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- .../frontend/protocol-field-handling.md | 6 +- .../src/protocols/openai/chat_completions.rs | 2 +- lib/llm/src/protocols/openai/validate.rs | 119 ++++++++++++++++-- .../frontend_protocol_validation_http.rs | 55 ++++++++ 4 files changed, 169 insertions(+), 13 deletions(-) diff --git a/docs/fern/pages/developer-guide/knowledge-base/modular-components/frontend/protocol-field-handling.md b/docs/fern/pages/developer-guide/knowledge-base/modular-components/frontend/protocol-field-handling.md index a88408ec9f95..2ced7e05e274 100644 --- a/docs/fern/pages/developer-guide/knowledge-base/modular-components/frontend/protocol-field-handling.md +++ b/docs/fern/pages/developer-guide/knowledge-base/modular-components/frontend/protocol-field-handling.md @@ -174,8 +174,10 @@ commitment. Chat completion and completion requests currently capture extra top-level fields in an internal map named `unsupported_fields`. The name describes the capture mechanism, not the final admission -decision. A small named set is accepted and validated for downstream handling. Other fields are -rejected by default. When `DYN_IGNORE_OPENAI_FE_UNSUPPORTED_FIELDS` is truthy, those other fields +decision. A small named set is accepted and validated for downstream handling. Chat completions +also accepts OpenAI's `safety_identifier`, `prompt_cache_retention`, and `verbosity`, validates +their values, and drops them, because Dynamo does not implement OpenAI's moderation, prompt cache +retention, or verbosity behavior. Other fields are rejected by default. When `DYN_IGNORE_OPENAI_FE_UNSUPPORTED_FIELDS` is truthy, those other fields are ignored and dropped; the switch does not enable general forwarding. See the [frontend configuration reference](../../../../reference/components/frontend-configuration.mdx) for the exact switch behavior. diff --git a/lib/llm/src/protocols/openai/chat_completions.rs b/lib/llm/src/protocols/openai/chat_completions.rs index 6b5d9c69f168..3b46654f2912 100644 --- a/lib/llm/src/protocols/openai/chat_completions.rs +++ b/lib/llm/src/protocols/openai/chat_completions.rs @@ -709,7 +709,7 @@ impl OpenAIOutputOptionsProvider for NvCreateChatCompletionRequest { impl ValidateRequest for NvCreateChatCompletionRequest { fn validate(&self) -> Result<(), anyhow::Error> { validate::validate_no_custom_tools(&self.inner)?; - validate::validate_no_unsupported_fields(&self.unsupported_fields)?; + validate::validate_no_unsupported_chat_fields(&self.unsupported_fields)?; validate::validate_guided_decoding(self)?; validate::validate_chat_template_args(self.chat_template_args.as_ref())?; validate::validate_messages(&self.inner.messages)?; diff --git a/lib/llm/src/protocols/openai/validate.rs b/lib/llm/src/protocols/openai/validate.rs index 2cba46968ca9..11cca9419c9a 100644 --- a/lib/llm/src/protocols/openai/validate.rs +++ b/lib/llm/src/protocols/openai/validate.rs @@ -125,6 +125,15 @@ pub const PASSTHROUGH_EXTRA_FIELDS: &[&str] = &[ "vllm_xargs", ]; +/// OpenAI Chat Completions fields that are accepted and dropped. They configure +/// OpenAI's moderation, prompt cache retention, and response verbosity, which +/// Dynamo does not implement. +pub const CHAT_IGNORED_FIELDS: &[&str] = + &["prompt_cache_retention", "safety_identifier", "verbosity"]; + +/// Maximum `safety_identifier` length in the OpenAI request contract. +const MAX_SAFETY_IDENTIFIER_LENGTH: usize = 128; + /// `vllm_xargs` keys vLLM itself fills from dedicated request fields. Dynamo only /// forwards router-generated `kv_transfer_params`, so clients may not set them. const RESERVED_VLLM_XARGS_KEYS: &[&str] = &["kv_transfer_params", "ec_transfer_params"]; @@ -158,17 +167,35 @@ pub fn validate_no_unsupported_fields( ) -> Result<(), anyhow::Error> { validate_no_unsupported_fields_with_ignore( unsupported_fields, + &[], *IGNORE_OPENAI_FE_UNSUPPORTED_FIELDS, ) } +/// Chat Completions version of [`validate_no_unsupported_fields`] that also +/// accepts [`CHAT_IGNORED_FIELDS`] after checking their values. +pub fn validate_no_unsupported_chat_fields( + unsupported_fields: &std::collections::HashMap, +) -> Result<(), anyhow::Error> { + validate_no_unsupported_fields_with_ignore( + unsupported_fields, + CHAT_IGNORED_FIELDS, + *IGNORE_OPENAI_FE_UNSUPPORTED_FIELDS, + )?; + validate_chat_ignored_fields(unsupported_fields) +} + fn validate_no_unsupported_fields_with_ignore( unsupported_fields: &std::collections::HashMap, + accepted_fields: &[&str], ignore_unsupported_fields: bool, ) -> Result<(), anyhow::Error> { let unknown: Vec<_> = unsupported_fields .keys() - .filter(|k| !PASSTHROUGH_EXTRA_FIELDS.contains(&k.as_str())) + .filter(|k| { + !PASSTHROUGH_EXTRA_FIELDS.contains(&k.as_str()) + && !accepted_fields.contains(&k.as_str()) + }) .map(|s| format!("`{}`", s)) .collect(); if !unknown.is_empty() && !ignore_unsupported_fields { @@ -214,6 +241,34 @@ fn validate_no_unsupported_fields_with_ignore( Ok(()) } +/// Checks [`CHAT_IGNORED_FIELDS`] values against the OpenAI request contract. +fn validate_chat_ignored_fields( + fields: &std::collections::HashMap, +) -> Result<(), anyhow::Error> { + if let Some(value) = fields.get("safety_identifier") + && !value.is_null() + && value + .as_str() + .is_none_or(|id| id.chars().count() > MAX_SAFETY_IDENTIFIER_LENGTH) + { + anyhow::bail!( + "`safety_identifier` must be a string of at most {MAX_SAFETY_IDENTIFIER_LENGTH} characters" + ); + } + for (field, allowed) in [ + ("prompt_cache_retention", &["in_memory", "24h"][..]), + ("verbosity", &["low", "medium", "high"][..]), + ] { + if let Some(value) = fields.get(field) + && !value.is_null() + && !value.as_str().is_some_and(|v| allowed.contains(&v)) + { + anyhow::bail!("`{field}` must be one of: {}", allowed.join(", ")); + } + } + Ok(()) +} + /// Validates `vllm_xargs` against vLLM's OpenAI schema: an object whose values are /// strings, numbers or booleans, or arrays of those. fn validate_vllm_xargs(value: &serde_json::Value) -> Result<(), anyhow::Error> { @@ -1379,7 +1434,7 @@ mod tests { "vllm_xargs": {"flag": true}, })) .unwrap(); - validate_no_unsupported_fields_with_ignore(&fields.fields, false).unwrap(); + validate_no_unsupported_fields_with_ignore(&fields.fields, &[], false).unwrap(); assert_eq!(fields.fields.len(), PASSTHROUGH_EXTRA_FIELDS.len()); assert_eq!(fields.fields["detokenize"], json!(false)); assert_eq!(fields.fields["stop_token_ids"], json!([])); @@ -1397,7 +1452,7 @@ mod tests { let fields: ExtraFields = serde_json::from_value(json!({field: bad})).unwrap(); assert_eq!(fields.fields[field], bad); let err = - validate_no_unsupported_fields_with_ignore(&fields.fields, false).unwrap_err(); + validate_no_unsupported_fields_with_ignore(&fields.fields, &[], false).unwrap_err(); assert!(err.to_string().contains(field), "{err}"); } } @@ -1405,14 +1460,14 @@ mod tests { #[test] fn validate_no_unsupported_fields_accepts_logprob_token_ids() { let fields = HashMap::from([("logprob_token_ids".to_string(), json!([14, 15]))]); - validate_no_unsupported_fields_with_ignore(&fields, false).unwrap(); + validate_no_unsupported_fields_with_ignore(&fields, &[], false).unwrap(); } #[test] fn validate_no_unsupported_fields_rejects_malformed_logprob_token_ids() { for bad in [json!(["notanint"]), json!(7), json!([[1, 2]]), json!([-1])] { let fields = HashMap::from([("logprob_token_ids".to_string(), bad)]); - let err = validate_no_unsupported_fields_with_ignore(&fields, false).unwrap_err(); + let err = validate_no_unsupported_fields_with_ignore(&fields, &[], false).unwrap_err(); assert!(err.to_string().contains("must be an array of token IDs")); } } @@ -1436,7 +1491,7 @@ mod tests { "tag": "read", }), )]); - validate_no_unsupported_fields_with_ignore(&fields, false).unwrap(); + validate_no_unsupported_fields_with_ignore(&fields, &[], false).unwrap(); } #[test] @@ -1457,20 +1512,21 @@ mod tests { ), ] { let fields = HashMap::from([("vllm_xargs".to_string(), bad)]); - let err = validate_no_unsupported_fields_with_ignore(&fields, false).unwrap_err(); + let err = validate_no_unsupported_fields_with_ignore(&fields, &[], false).unwrap_err(); assert!(err.to_string().contains(message), "{err}"); } } #[test] fn validate_no_unsupported_fields_rejects_unknown_fields_by_default() { - let err = validate_no_unsupported_fields_with_ignore(&unknown_fields(), false).unwrap_err(); + let err = + validate_no_unsupported_fields_with_ignore(&unknown_fields(), &[], false).unwrap_err(); assert!(err.to_string().contains("Unsupported parameter(s)")); } #[test] fn validate_no_unsupported_fields_ignores_unknown_fields_when_configured() { - validate_no_unsupported_fields_with_ignore(&unknown_fields(), true).unwrap(); + validate_no_unsupported_fields_with_ignore(&unknown_fields(), &[], true).unwrap(); } #[test] @@ -1481,10 +1537,53 @@ mod tests { ]); let err = - validate_no_unsupported_fields_with_ignore(&unsupported_fields, true).unwrap_err(); + validate_no_unsupported_fields_with_ignore(&unsupported_fields, &[], true).unwrap_err(); assert!(err.to_string().contains("stop_token_ids")); } + #[test] + fn chat_ignored_fields_are_accepted_only_by_chat() { + let fields = HashMap::from([ + ( + "safety_identifier".to_string(), + json!("é".repeat(MAX_SAFETY_IDENTIFIER_LENGTH)), + ), + ("prompt_cache_retention".to_string(), json!("24h")), + ("verbosity".to_string(), json!("low")), + ]); + validate_no_unsupported_fields_with_ignore(&fields, CHAT_IGNORED_FIELDS, false).unwrap(); + validate_chat_ignored_fields(&fields).unwrap(); + let err = validate_no_unsupported_fields_with_ignore(&fields, &[], false).unwrap_err(); + assert!( + err.to_string().contains("Unsupported parameter(s)"), + "{err}" + ); + + let nulls = CHAT_IGNORED_FIELDS + .iter() + .map(|field| (field.to_string(), serde_json::Value::Null)) + .collect::>(); + validate_chat_ignored_fields(&nulls).unwrap(); + } + + #[test] + fn chat_ignored_fields_reject_malformed_values() { + for (field, bad) in [ + ("safety_identifier", json!(7)), + ( + "safety_identifier", + json!("x".repeat(MAX_SAFETY_IDENTIFIER_LENGTH + 1)), + ), + ("prompt_cache_retention", json!("1h")), + ("verbosity", json!("loud")), + ("verbosity", json!(["low"])), + ] { + let fields = HashMap::from([(field.to_string(), bad)]); + let err = validate_chat_ignored_fields(&fields).unwrap_err(); + assert!(err.to_string().contains(field), "{err}"); + } + } + #[test] fn validate_top_p_rejects_zero() { let err = validate_top_p(Some(0.0)).unwrap_err(); diff --git a/lib/llm/tests/frontend_protocol_validation_http.rs b/lib/llm/tests/frontend_protocol_validation_http.rs index 3f65d1753bdb..828aa198dc41 100644 --- a/lib/llm/tests/frontend_protocol_validation_http.rs +++ b/lib/llm/tests/frontend_protocol_validation_http.rs @@ -1220,6 +1220,61 @@ async fn responses_auto_tool_choice_without_tools_reaches_backend() { .await; } +/// OpenAI's `safety_identifier`, `prompt_cache_retention`, and `verbosity` reach the +/// backend on chat completions without being forwarded. Legacy completions, whose +/// contract has no such fields, still rejects them. +#[tokio::test] +#[serial] +async fn chat_completions_accept_and_drop_openai_ignored_fields() { + const FIELDS: [(&str, &str); 3] = [ + ("safety_identifier", "user-1234"), + ("prompt_cache_retention", "24h"), + ("verbosity", "low"), + ]; + fn with_fields(mut body: Value) -> Value { + for (field, value) in FIELDS { + body[field] = json!(value); + } + body + } + + temp_env::async_with_vars(BASE_ENV, async { + let script = load_agent_fixture("text.sse").await.unwrap(); + let svc = HarnessService::start(vec![script; 2]).await; + + for stream in [false, true] { + let body = with_fields(json!({ + "model": MODEL, + "stream": stream, + "messages": [{"role": "user", "content": "ping"}], + })); + let response = post_json(&svc, "/v1/chat/completions", body).await; + assert_eq!(response.status(), reqwest::StatusCode::OK); + bounded(response.bytes()).await.unwrap(); + + let requests = svc.engine.take_requests().await; + assert_eq!(requests.len(), 1); + let forwarded = serde_json::to_value(&requests[0]).unwrap(); + for (field, _) in FIELDS { + assert!(forwarded.get(field).is_none(), "{field} was forwarded"); + } + } + + let body = with_fields(json!({"model": MODEL, "prompt": "ping"})); + let response = post_json(&svc, "/v1/completions", body).await; + assert_openai_error( + response, + ExpectedError::Validation, + "Unsupported parameter(s)", + ) + .await; + + assert_eq!(svc.engine.remaining_scripts().await, 0); + svc.shutdown().await; + }) + .await; +} + #[tokio::test] #[serial] async fn responses_reject_empty_input_and_required_tool_choice_without_tools() {