-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Refactor: Filtering beta header after transformation #23715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| import litellm | ||
| from litellm.anthropic_beta_headers_manager import ( | ||
| filter_and_transform_beta_headers, | ||
| update_request_with_filtered_beta, | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -116,6 +117,32 @@ def test_unknown_headers_filtered_out(self, provider): | |
| unknown not in filtered | ||
| ), f"Unknown header '{unknown}' should be filtered out for {provider}" | ||
|
|
||
| def test_update_request_with_filtered_beta_vertex_ai(self): | ||
| """Test combined filtering for both HTTP headers and request body betas.""" | ||
| headers = { | ||
| "anthropic-beta": "files-api-2025-04-14,context-management-2025-06-27,code-execution-2025-05-22" | ||
| } | ||
| request_data = { | ||
| "anthropic_beta": [ | ||
| "files-api-2025-04-14", | ||
| "context-management-2025-06-27", | ||
| "code-execution-2025-05-22", | ||
| ] | ||
| } | ||
|
|
||
| filtered_headers, filtered_request_data = update_request_with_filtered_beta( | ||
| headers=headers, | ||
| request_data=request_data, | ||
| provider="vertex_ai", | ||
| ) | ||
|
|
||
| assert ( | ||
| filtered_headers.get("anthropic-beta") == "context-management-2025-06-27" | ||
| ) | ||
| assert filtered_request_data.get("anthropic_beta") == [ | ||
| "context-management-2025-06-27" | ||
| ] | ||
|
Comment on lines
+120
to
+144
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test covers utility, not the actual bug path The new test validates
Without this, a future refactor that re-introduces the same omission would not be caught by the test suite. |
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_anthropic_messages_http_headers_filtering(self): | ||
| """Test that Anthropic messages API filters HTTP headers correctly.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing type guard for
anthropic_betastring valuefilter_and_transform_beta_headersexpects aList[str], butrequest_data["anthropic_beta"]could be a plain comma-separated string if the caller passes it throughoptional_paramsdirectly. If it is a string,for header in beta_headers:will iterate over individual characters ("f","i","l","e", …) — none of which exist in the provider mapping — silently dropping all beta headers.This is unlikely to occur in the current Vertex AI path (where
transform_requestalways stores it as alist), but the function is a public utility and acceptsdictwithout type constraints, making it brittle for future callers.