Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions lib/llm/src/http/service/openai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,9 @@ impl ErrorMessage {
)
}

/// Unsupported multimodal content is a client error: no retry can make the
/// request succeed, and infrastructure above the frontend counts 5xx as a
/// Unsupported multimodal content or a deprecated request field such as
/// `function_call` is a client error: no retry can make the request
/// succeed, and infrastructure above the frontend counts 5xx as a
Comment on lines +723 to +725

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up idea: the same reasoning covers /v1/responses, where background, previous_response_id, prompt, max_tool_calls, and the RL nvext.extra_fields still return 501 from validate_response_unsupported_fields, and openai-python retries every 5xx twice. Worth a separate PR?

/// server-side fault. Answered 400 where `not_implemented_error` answers 501.
/// The legacy request metric remains `NotImplemented` to distinguish an
/// unsupported feature from malformed input.
Expand Down Expand Up @@ -3552,10 +3553,10 @@ async fn chat_completions(
return Err(err_response);
}

// Handle unsupported fields - if Some(resp) is returned by
// Handle unsupported fields - if Err(resp) is returned by
// validate_chat_completion_unsupported_fields,
// then a field was used that is unsupported. We will log an error message
// and early return a 501 NOT_IMPLEMENTED status code. Otherwise, proceeed.
// then a field was used that is unsupported. We early return a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the four-line comment above this validation call. It narrates the immediately following if let Err and early return; the 400 status and its rationale are already documented on unsupported_content_error. It adds no lasting constraint.

🤖 AI Fix

Delete the four-line comment beginning Handle unsupported fields above validate_chat_completion_unsupported_fields(&request).

// 400 BAD_REQUEST status code. Otherwise, proceed.
if let Err(err_response) = validate_chat_completion_unsupported_fields(&request) {
inflight_guard.mark_error(extract_error_type_from_response(&err_response));
return Err(err_response);
Expand Down Expand Up @@ -3900,22 +3901,22 @@ async fn chat_completions(
}

/// Checks for unsupported fields in the request.
/// Returns Some(response) if unsupported fields are present.
/// Returns an error response if unsupported fields are present.
#[allow(deprecated)]
pub fn validate_chat_completion_unsupported_fields(
request: &NvCreateChatCompletionRequest,
) -> Result<(), ErrorResponse> {
let inner = &request.inner;

if inner.function_call.is_some() {
return Err(ErrorMessage::not_implemented_error(
return Err(ErrorMessage::unsupported_content_error(
VALIDATION_PREFIX.to_string()
+ "`function_call` is deprecated. Please migrate to use `tool_choice` instead.",
));
}

if inner.functions.is_some() {
return Err(ErrorMessage::not_implemented_error(
return Err(ErrorMessage::unsupported_content_error(
VALIDATION_PREFIX.to_string()
+ "`functions` is deprecated. Please migrate to use `tools` instead.",
));
Expand Down
52 changes: 52 additions & 0 deletions lib/llm/tests/frontend_protocol_validation_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,58 @@ async fn chat_completions_reject_empty_tools() {
.await;
}

/// The deprecated `function_call` and `functions` fields answer 400 rather than
/// 501, so OpenAI SDKs do not retry them as server errors. The request metric
/// keeps the `NotImplemented` label.
#[tokio::test]
#[serial]
async fn chat_completions_reject_deprecated_function_fields_as_bad_request() {
temp_env::async_with_vars(BASE_ENV, async {
let svc = HarnessService::start(Vec::new()).await;

for (field, value, message) in [
(
"function_call",
json!({"name": "get_weather"}),
"`function_call` is deprecated. Please migrate to use `tool_choice` instead.",
),
(
"functions",
json!([{"name": "get_weather", "parameters": {"type": "object"}}]),
"`functions` is deprecated. Please migrate to use `tools` instead.",
),
] {
for stream in [false, true] {
let mut body = json!({
"model": MODEL,
"stream": stream,
"messages": [{"role": "user", "content": "ping"}],
});
body[field] = value.clone();
let response = post_json(&svc, "/v1/chat/completions", body).await;
assert_openai_error(response, ExpectedError::UnsupportedContent, message).await;
}
}

for request_type in [RequestType::Unary, RequestType::Stream] {
assert_error_metrics(
&svc,
&Endpoint::ChatCompletions,
&request_type,
&[
(ErrorType::NotImplemented, 2),
(ErrorType::Validation, 0),
(ErrorType::Internal, 0),
],
);
}

assert!(svc.engine.take_requests().await.is_empty());
svc.shutdown().await;
})
.await;
}

#[tokio::test]
#[serial]
async fn anthropic_content_validation_applies_to_messages_and_count_tokens() {
Expand Down
Loading