Repository navigation
refactor(proxy): expose public names for private proxy helpers - #45170
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
@greptileai review |
|
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review |
| from litellm.proxy.management_endpoints.policy_endpoints.endpoints import ( # noqa: F401 | ||
| _build_all_names_per_competitor, | ||
| _build_comparison_blocked_words, | ||
| _build_competitor_guardrail_definitions, | ||
| _build_name_blocked_words, | ||
| _build_recommendation_blocked_words, | ||
| _build_refinement_prompt, | ||
| _clean_competitor_line, | ||
| _parse_variations_response, | ||
| build_all_names_per_competitor, | ||
| build_comparison_blocked_words, | ||
| build_competitor_guardrail_definitions, | ||
| build_name_blocked_words, | ||
| build_recommendation_blocked_words, | ||
| build_refinement_prompt, | ||
| clean_competitor_line, | ||
| parse_variations_response, | ||
| ) |
There was a problem hiding this comment.
Existing imports such as from litellm.proxy.management_endpoints.policy_endpoints import _build_refinement_prompt now fail, stopping the caller's code from loading. The wildcard import excludes private names, and this list now exports only public names. Keeping aliases in endpoints.py does not preserve the package imports
Keep both names here. Dropping the old exports violates the repository directive against backwards-incompatible changes without user-controlled flags
| from litellm.proxy.management_endpoints.policy_endpoints.endpoints import ( # noqa: F401 | |
| _build_all_names_per_competitor, | |
| _build_comparison_blocked_words, | |
| _build_competitor_guardrail_definitions, | |
| _build_name_blocked_words, | |
| _build_recommendation_blocked_words, | |
| _build_refinement_prompt, | |
| _clean_competitor_line, | |
| _parse_variations_response, | |
| build_all_names_per_competitor, | |
| build_comparison_blocked_words, | |
| build_competitor_guardrail_definitions, | |
| build_name_blocked_words, | |
| build_recommendation_blocked_words, | |
| build_refinement_prompt, | |
| clean_competitor_line, | |
| parse_variations_response, | |
| ) | |
| from litellm.proxy.management_endpoints.policy_endpoints.endpoints import ( # noqa: F401 | |
| _build_all_names_per_competitor, | |
| _build_comparison_blocked_words, | |
| _build_competitor_guardrail_definitions, | |
| _build_name_blocked_words, | |
| _build_recommendation_blocked_words, | |
| _build_refinement_prompt, | |
| _clean_competitor_line, | |
| _parse_variations_response, | |
| build_all_names_per_competitor, | |
| build_comparison_blocked_words, | |
| build_competitor_guardrail_definitions, | |
| build_name_blocked_words, | |
| build_recommendation_blocked_words, | |
| build_refinement_prompt, | |
| clean_competitor_line, | |
| parse_variations_response, | |
| ) |
Rule Used: What: avoid backwards-incompatible changes without user-controlled flags Why: This breaks current behaviour for users using existing functionality Example of BAD: this PR (#22164) introduced run_post_custom... (source)
There was a problem hiding this comment.
Agreed. Restoring the old private names next to the public ones in this and the hooks package export, with an import test
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review |
|
bugbot run |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…_proxy Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> # Conflicts: # litellm/proxy/management_endpoints/common_utils.py # litellm/proxy/management_endpoints/key_management_endpoints.py # litellm/proxy/management_endpoints/team_endpoints.py # tests/unit/proxy/auth/test_route_checks.py # tests/unit/proxy/management_endpoints/test_common_utils.py
…_proxy Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> # Conflicts: # litellm/proxy/utils.py # tests/unit/proxy/_experimental/mcp_server/test_mcp_guardrail_usage_monitor.py # tests/unit/proxy/_experimental/mcp_server/test_mcp_server_manager.py # tests/unit/proxy/hooks/test_parallel_request_limiter_v3.py
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review |
| @@ -129,7 +129,7 @@ def test_hash_token_if_needed_handles_sk_prefix() -> None: | |||
|
|
|||
| def test_hash_token_if_needed_error_on_non_string() -> None: | |||
| with pytest.raises(AttributeError): | |||
| _hash_token_if_needed(None) # type: ignore[arg-type] | |||
| hash_token_if_needed(None) # type: ignore[arg-type] | |||
There was a problem hiding this comment.
The updated call retains # type: ignore[arg-type], which pyright ignores. AGENTS.md bans this syntax and requires an exact pyright rule with a reason. Replace it with # pyright: ignore[reportArgumentType] and explain that the invalid input is intentional. This repository requirement must be satisfied before merging
| hash_token_if_needed(None) # type: ignore[arg-type] | |
| hash_token_if_needed(None) # pyright: ignore[reportArgumentType] # intentional invalid input checks the error |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Agreed. Switching it to an exact reportArgumentType suppression with a reason, since the None input is intentional
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review |
|
bugbot run |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 46b4328. Configure here.
…ermission's payload Upstream PR BerriAI#45170 retyped set_object_permission's data_json parameter and return value as dict[str, object], which silently broke basedpyright's argument-compatibility checking at its three call sites (key, user, and team creation), each of which unpacks the dict as **kwargs into generate_key_helper_fn's dozens of concretely-typed parameters. object is a real, narrow type that must be checked; the untyped dict the function had before upstream's change was treated permissively, so the incompatibility never surfaced. Any is the correct type for this JSON-like payload destined for **kwargs unpacking, and restores the previous permissive behavior without widening the function's own contract. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGP2qHGkEN9qza1tM2AswG
Upstream BerriAI#45170 makes _read_request_body and _get_metadata_variable_name public. The PR call sites use the new names. Upstream BerriAI#40193 gates every lint rule at its merge-base count. The PR lines now carry mutable-ok and rebind-ok markers with reasons. filter_pinned_deployment is generic over the deployment type. The new helpers read untyped dicts through a pydantic TypeAdapter. # Conflicts: # litellm/proxy/common_request_processing.py # litellm/proxy/video_endpoints/endpoints.py
…-to-org-config Resolve the user search check against the user_api_key_has_admin_view rename from BerriAI#45170 Keep UI_SETTINGS_CACHE_KEY and UI_SETTINGS_CACHE_TTL, which moyai_endpoints now imports for its own cache write
TLDR
Problem this solves:
reportPrivateUsageon main is 800, and 613 of those hits point atlitellm/proxy/symbols_privatehelpersHow it solves it:
_namestays as an alias to the same object (_should_return_raw_model_name: Final = should_return_raw_model_name,_handle_llm_api_exception = handle_llm_api_exceptionin the class body), so existing imports and patches keep working_PROXY_*style definitions and the public name is the alias (PROXY_MaxParallelRequestsHandler_v3 = _PROXY_MaxParallelRequestsHandler_v3), because__name__drives the no-log callback filter, internal-callback checks, and health and guardrail names_PROXY_LiteLLMManagedFilesand_PROXY_LiteLLMManagedVectorStores, which refactor: expose core private helpers under public names #44871 had renamed so their class names lost_PROXY_on mainhooksandpolicy_endpointspackages), sofrom litellm.proxy.proxy_server import _ProxyDBLoggerkeeps working_can_object_call_modelkeeps its private definition with a public alias, since the recursive-function check lists it by nameProxyLogging.convert_mcp_to_llm_formatis a public forwarder that calls the old private method, since hooks may override itreportPrivateUsagesuppressions with a reason_license_check(a rebound global), names whose public twin has a different contract likeProxyConfig._add_deployment, and helpers whose callers live inenterprise/: Final(175) or# rebind-ok(65) the type-discipline gate now checks on changed lines, and the 63 newly public functions got the return type basedpyright already inferred, includingNeverfor raise-only helpersreportPrivateUsagegoes from 800 to 258 and no basedpyright rule goes upUser Flow
Before: a proxy user sends LLM and management requests and gets normal responses
from litellm.proxy.utils import _check_and_merge_model_level_guardrailsworksAfter: the same requests behave the same, and the public names are also available
check_and_merge_model_level_guardrailsPre-Submission checklist
Screenshots / Proof of Fix
Two proxies on the same config, real Postgres, and real provider keys, Before at main
3538e87and After at this branch, each sent the same 18 requestsModel id headers and response shapes matched in every case. The Vertex 404 is the project lacking that model, identical on both sides
tests/unit/test_private_usage_aliases.pychecks alias identity for every renamed proxy symbol and that the forwarder reaches a subclass override of the private name with the exact arguments. Swapping two arguments in the forwarder makes it failType
🧹 Refactoring
Caveats (if any)
Low
ProxyBaseLLMRequestProcessingthat overrides the old_nameis no longer reached through those callers. None of the 216 aliased methods are on documented extension points likeCustomLoggerorCustomGuardrailLink to Devin session: https://app.devin.ai/sessions/2ca967be4d764e32bf06b907fdafcf4a
Open in Devin Desktop: https://app.devin.ai/desktop/session/2ca967be4d764e32bf06b907fdafcf4a?variant=devin
Requested by: @mateo-berri
Note
Low Risk
Behavior-preserving renames and alias bindings; no auth or billing logic changes in the diff, only which symbol names production code imports.
Overview
This PR continues the proxy public API refactor by switching internal call sites from underscore-prefixed helpers to their public twins, while keeping the old names as module-level aliases (often with
Finaland legacyF401re-exports) so existing imports and patches still resolve to the same objects.Enterprise hooks:
PROXY_LiteLLMManagedFilesandPROXY_LiteLLMManagedVectorStoresare defined again as_PROXY_*classes with public aliases at module bottom, matching the callback-registry__name__pattern used for dynamic rate limiters (PROXY_DynamicRateLimitHandlerincustom_logger_registry/litellm_logging).Cross-cutting renames in this diff: file-id helpers (
is_base64_encoded_unified_file_id), spend reporting (get_spend_report_for_time_range), auth/budget checks (premium_user_check,team_max_budget_check,can_object_call_model,run_centralized_common_checks), request parsing (read_request_body,safe_get_request_headers), passthrough streaming (route_streaming_logging_to_handler), and Prometheus key listing (list_key_helper).MCP / OAuth surface (largest footprint):
MCPRequestHandlerheader/body helpers, bridge mint/refresh (prepare_bridge_mint,finish_bridge_mint,bridge_mint_error_response), token TTL (compute_per_user_token_ttl), encryption (get_salt_key,V2_GCM_PREFIX), OpenAPI context vars (request_auth_header, etc.), andMCPServerManagermethods (get_tools_from_server,should_strip_caller_authorization, …) are called by public names; private names remain as aliases on the defining modules.Reviewed by Cursor Bugbot for commit 46b4328. Bugbot is set up for automated code reviews on this repo. Configure here.