Repository navigation
fix(frontend): return 400 for deprecated function_call and functions - #17507
mananpatelll wants to merge 1 commit into
Conversation
OpenAI SDKs retry 5xx, so the 501 made older clients retry a request that can never succeed. The error messages and the NotImplemented request metric stay the same. Signed-off-by: Manan Patel <144215944+mananpatelll@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
|
👋 Hi mananpatelll! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
Bot
added
fix
external-contribution
github-actions
Bot
added
the
frontend
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
400 handling and regression coverage lib/llm/src/http/service/openai.rs, lib/llm/tests/frontend_protocol_validation_http.rs |
The validator routes deprecated function_call and functions fields to unsupported-content errors with HTTP 400 responses and the NotImplemented metric classification. Tests cover unary and streaming requests, field-specific messages, metrics, and engine request counts. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Merge Risk: 
3d5b2
-
Deprecated function fields are intended to return 400 without reaching the engine. No merge-blocking concern is established.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: returning HTTP 400 for deprecated function_callandfunctionsfields.Description check The description includes all required sections, explains the problem and implementation, documents validation results, identifies ongoing work, and includes the checklist. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
| // 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 |
There was a problem hiding this comment.
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).
Overview
Chat completions returns 501 for requests with the deprecated
function_callorfunctionsfields. OpenAI SDKs retry 5xx, so older clients keep retrying a request that can never succeed. This makes it a 400 with the same message.Key Changes
2 lines of core logic.
openai.rs:validate_chat_completion_unsupported_fieldsusesunsupported_content_errorinstead ofnot_implemented_error. That's the existing 400 helper for unsupported features (Responses uses it for unsupported content), and it keeps the request metric atNotImplemented. The failure counter now saysinvalid_requestinstead ofnot_implementedfor these requests.frontend_protocol_validation_http.rs: new test.Validation
function_callset: 501 before, 400 now, streaming or not.functionsset: 501 before, 400 now, streaming or not.chat_completions_reject_deprecated_function_fields_as_bad_requestsends both fields with and without streaming and checks the status, message, and metric. It fails onmainwith a 501. Ran on macOS with--no-default-features.Issues and Ongoing Work
Pull Request Checklist
/pr-checklistskill.🤖 Generated with Claude Code
Summary by CodeRabbit
function_callorfunctionsfields now receive a clear HTTP 400 response, including in streaming requests. These requests are rejected before processing, with a message identifying the deprecated field.