diff --git a/.github/workflows/test-linting.yml b/.github/workflows/test-linting.yml index 5384e873f22..730418d59cd 100644 --- a/.github/workflows/test-linting.yml +++ b/.github/workflows/test-linting.yml @@ -213,12 +213,9 @@ jobs: tests/code_coverage_tests/test_provider_replay_harness.py \ tests/code_coverage_tests/test_provider_cache.py - - name: Check for circular imports + - name: Check layer imports if: steps.changes.outputs.decision != 'skip' - run: | - cd litellm - uv run --no-sync python ../tests/documentation_tests/test_circular_imports.py - cd .. + run: uv run --no-sync python scripts/check_layer_imports.py --base "$GATE_BASE_SHA" - name: Check import safety if: steps.changes.outputs.decision != 'skip' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0af12bd5318..ea1e703bc94 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -151,7 +151,7 @@ Individual linting commands: make format-check # Check ruff format formatting make lint-ruff # Run Ruff linting make lint-basedpyright # Run basedpyright type checking -make check-circular-imports # Check for circular imports +make check-layer-imports # Check SDK/proxy layer import rules make check-import-safety # Check import safety ``` diff --git a/Makefile b/Makefile index 1552187e906..e976f7878b3 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ lint-basedpyright lint-e2e-basedpyright lint-type-discipline \ lint-ruff-strict lint-gate lint-test-quality lint-openapi-docs \ install-dev install-proxy-dev install-test-deps install-hooks \ - install-helm-unittest check-circular-imports check-import-safety check check-inner pre-commit \ + install-helm-unittest check-layer-imports check-import-safety check check-inner pre-commit \ lint-install lint-fetch-base bootstrap # Default target @@ -27,7 +27,7 @@ help: @echo " make pre-commit - Legacy alias for make check" @echo " make format - Apply ruff format code formatting" @echo " make format-check - Check ruff format code formatting (matches CI)" - @echo " make lint - Run all linting (Ruff, basedpyright, format check, circular imports, import safety)" + @echo " make lint - Run all linting (Ruff, basedpyright, format check, layer imports, import safety)" @echo " make lint-ruff - Run Ruff linting only" @echo " make lint-basedpyright - Run basedpyright strict, gated by per-rule error counts" @echo " make lint-e2e-basedpyright - Run basedpyright over tests/e2e and tests/e2e_harness (zero errors allowed)" @@ -36,7 +36,7 @@ help: @echo " make lint-gate - Strict ruff gate in CI-parity mode (fetches the default branch, simulates the merge)" @echo " make lint-test-quality - Gate the test suite's TQ counts against their merge-base counts" @echo " make lint-openapi-docs - Gate undocumented POST routes in the proxy OpenAPI document against their merge-base counts" - @echo " make check-circular-imports - Check for circular imports" + @echo " make check-layer-imports - Check SDK/proxy layer import rules against scripts/layer_imports_allowlist.txt" @echo " make check-import-safety - Check import safety" @echo " make test - Run all tests" @echo " make test-unit - Run unit tests (tests/unit)" @@ -236,8 +236,8 @@ lint-ruff-strict: install-dev lint-gate: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE) $(UV_RUN) python scripts/ruff_strict_gate.py --base "$(BASE_REF)" -check-circular-imports: $(LINT_DEP_INSTALL) - cd litellm && $(UV_RUN) python ../tests/documentation_tests/test_circular_imports.py && cd .. +check-layer-imports: $(LINT_DEP_INSTALL) + $(UV_RUN) python scripts/check_layer_imports.py --base "$$($(RESOLVE_BASE))" check-import-safety: $(LINT_DEP_INSTALL) @$(UV_RUN) python -c "from litellm import *; print('[from litellm import *] OK! no issues!');" || (echo '🚨 import failed, this means you introduced unprotected imports! 🚨'; exit 1) @@ -245,7 +245,7 @@ check-import-safety: $(LINT_DEP_INSTALL) # Combined linting, isomorphic to test-linting.yml's python job so a local pass means a # green CI lint: it installs the same env (proxy-dev + generated Prisma client) and then # runs the diff-scoped ruff format check, whole-tree ruff check, the strict-rule / -# type-discipline / basedpyright gates as a delta vs the base, then the circular-import +# type-discipline / basedpyright gates as a delta vs the base, then the layer-import # and import-safety checks. Steps that compare against the base resolve it the same way CI # does (merge-base with origin's current default branch). Setup (env sync, Prisma client, # base fetch) runs once up front; the checks themselves are independent, so a sub-make @@ -257,10 +257,10 @@ lint-inner: lint-install @base_ref=$$($(RESOLVE_BASE)) && \ $(MAKE) BASE_REF="$$base_ref" -j $(LINT_JOBS) $(LINT_OUTPUT_SYNC) LINT_DEP_INSTALL= LINT_E2E_DEP_INSTALL= LINT_DEP_BASE= lint-checks -lint-checks: lint-format-check-changed lint-ruff lint-gate lint-type-discipline lint-test-quality lint-openapi-docs lint-basedpyright lint-e2e-basedpyright check-circular-imports check-import-safety +lint-checks: lint-format-check-changed lint-ruff lint-gate lint-type-discipline lint-test-quality lint-openapi-docs lint-basedpyright lint-e2e-basedpyright check-layer-imports check-import-safety # Faster linting for local development (only checks changed code) -lint-dev: lint-format-changed check-circular-imports check-import-safety +lint-dev: lint-format-changed check-layer-imports check-import-safety # Run the gating CI checks against your changes. Scopes to staged files when anything # is staged (warning about changed files left unstaged); with nothing staged it falls diff --git a/scripts/check_layer_imports.py b/scripts/check_layer_imports.py new file mode 100644 index 00000000000..2639769005d --- /dev/null +++ b/scripts/check_layer_imports.py @@ -0,0 +1,212 @@ +import argparse +import ast +import sys +from collections.abc import Callable, Iterable, Iterator, Sequence +from dataclasses import dataclass +from itertools import chain, product +from pathlib import Path +from types import MappingProxyType +from typing import Final, Literal + +from lint_base_counts import REPO_GIT, Git, resolve_base_point + +PACKAGE_ROOT: Final = Path("litellm") +ALLOWLIST: Final = Path("scripts/layer_imports_allowlist.txt") +LEAF_MODULES: Final = frozenset( + { + "litellm.litellm_core_utils.completion_timeout", + "litellm.litellm_core_utils.core_helpers", + "litellm.litellm_core_utils.duration_parser", + "litellm.litellm_core_utils.llm_response_utils.get_headers", + "litellm.litellm_core_utils.provider_affinity", + } +) +TYPES_LAYER: Final = ( + frozenset({"litellm.types", "litellm.models", "litellm.constants", "litellm._logging", "litellm._uuid"}) + | LEAF_MODULES +) +L2_FORBIDDEN: Final = ("litellm.proxy", "litellm.main", "litellm.router") + +Context = Literal["module", "function"] + + +@dataclass(frozen=True, slots=True) +class Rule: + name: str + applies_to: Callable[[str], bool] + forbids: Callable[[str], bool] + + +@dataclass(frozen=True, slots=True) +class Edge: + path: str + importer: str + imported: str + context: Context + line: int + + +@dataclass(frozen=True, slots=True) +class Violation: + rule: str + edge: Edge + + @property + def key(self) -> str: + return f"{self.rule} {self.edge.context} {self.edge.path} {self.edge.imported}" + + +def is_under(module: str, package: str) -> bool: + return module == package or module.startswith(f"{package}.") + + +def escapes_types_layer(module: str) -> bool: + return is_under(module, "litellm") and not any(is_under(module, allowed) for allowed in TYPES_LAYER) + + +RULES: Final = ( + Rule( + "L1", + lambda module: is_under(module, "litellm") and not is_under(module, "litellm.proxy"), + lambda target: is_under(target, "litellm.proxy"), + ), + Rule( + "L2", + lambda module: is_under(module, "litellm.llms"), + lambda target: any(is_under(target, forbidden) for forbidden in L2_FORBIDDEN), + ), + Rule("L3", lambda module: is_under(module, "litellm.types"), escapes_types_layer), + Rule("LEAF", lambda module: module in LEAF_MODULES, escapes_types_layer), +) + + +def module_name(path: Path) -> str: + parts: Final = path.with_suffix("").parts + return ".".join(parts[:-1] if parts[-1] == "__init__" else parts) + + +def is_type_checking(test: ast.expr) -> bool: + return (isinstance(test, ast.Name) and test.id == "TYPE_CHECKING") or ( + isinstance(test, ast.Attribute) and test.attr == "TYPE_CHECKING" + ) + + +def import_nodes(node: ast.AST, context: Context) -> Iterator[tuple[ast.Import | ast.ImportFrom, Context]]: + if isinstance(node, ast.Import | ast.ImportFrom): + yield node, context + return + children: Final[Iterable[ast.AST]] = ( + node.orelse if isinstance(node, ast.If) and is_type_checking(node.test) else ast.iter_child_nodes(node) + ) + child_context: Final[Context] = ( + "function" if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.Lambda) else context + ) + for child in children: + yield from import_nodes(child, child_context) + + +def imported_modules( + node: ast.Import | ast.ImportFrom, importer: str, is_package: bool, known: frozenset[str] +) -> tuple[str, ...]: + if isinstance(node, ast.Import): + return tuple(alias.name for alias in node.names) + package: Final = importer.split(".") if is_package else importer.split(".")[:-1] + anchor: Final = tuple(package[: len(package) - node.level + 1]) if node.level else () + base: Final = ".".join((*anchor, *((node.module,) if node.module else ()))) + return tuple(f"{base}.{alias.name}" if f"{base}.{alias.name}" in known else base for alias in node.names) + + +def package_prefixes(module: str) -> Iterator[str]: + parts: Final = module.split(".") + return (".".join(parts[:end]) for end in range(1, len(parts) + 1)) + + +def known_modules(paths: Iterable[Path]) -> frozenset[str]: + return frozenset(chain.from_iterable(package_prefixes(module_name(path)) for path in paths)) + + +def file_edges(path: Path, known: frozenset[str]) -> Iterator[Edge]: + importer: Final = module_name(path) + tree: Final = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + for node, context in import_nodes(tree, "module"): + for imported in imported_modules(node, importer, path.name == "__init__.py", known): + yield Edge(path.as_posix(), importer, imported, context, node.lineno) + + +def violations(paths: tuple[Path, ...]) -> tuple[Violation, ...]: + known: Final = known_modules(paths) + edges: Final = tuple(chain.from_iterable(file_edges(path, known) for path in paths)) + return tuple( + Violation(rule.name, edge) + for rule, edge in product(RULES, edges) + if rule.applies_to(edge.importer) and rule.forbids(edge.imported) + ) + + +def parse_allowlist(text: str) -> frozenset[str]: + lines: Final = (" ".join(line.split()) for line in text.splitlines()) + return frozenset(line for line in lines if line and not line.startswith("#")) + + +def moved_into_function(key: str, base_allowed: frozenset[str]) -> bool: + rule, context, rest = key.split(" ", 2) + return context == "function" and f"{rule} module {rest}" in base_allowed + + +def report( + found: tuple[Violation, ...], allowed: frozenset[str], base_allowed: frozenset[str] | None +) -> tuple[str, ...]: + first_seen: Final = MappingProxyType({violation.key: violation for violation in reversed(found)}) + new: Final = tuple( + f"{violation.edge.path}:{violation.edge.line}: {violation.rule} forbids " + f"{violation.edge.importer} importing {violation.edge.imported} ({violation.edge.context} level)" + for key, violation in sorted(first_seen.items()) + if key not in allowed + ) + stale: Final = tuple( + f"{ALLOWLIST}: stale entry '{key}' no longer matches an import, delete it" + for key in sorted(allowed - first_seen.keys()) + ) + added: Final = ( + tuple( + f"{ALLOWLIST}: entry '{key}' is not in the base allowlist, fix the import instead of allowlisting it" + for key in sorted(allowed - base_allowed) + if not moved_into_function(key, base_allowed) + ) + if base_allowed is not None + else () + ) + return new + stale + added + + +def git_base_allowlist(ref: str, git: Git = REPO_GIT) -> frozenset[str] | None: + base_point: Final = resolve_base_point(ref, git) + tracked: Final = git(["ls-tree", "--name-only", base_point, "--", ALLOWLIST.as_posix()]).strip() + return parse_allowlist(git(["show", f"{base_point}:{ALLOWLIST.as_posix()}"])) if tracked else None + + +class Arguments(argparse.Namespace): + base: str | None = None + + +def main( + argv: Sequence[str] | None = None, + base_allowlist: Callable[[str], frozenset[str] | None] = git_base_allowlist, +) -> int: + parser: Final = argparse.ArgumentParser(description="Check SDK/proxy layer import rules") + parser.add_argument( + "--base", help="Also reject allowlist entries missing from the allowlist at the merge base with this ref" + ) + args: Final = parser.parse_args(argv, namespace=Arguments()) + paths: Final = tuple(sorted(PACKAGE_ROOT.rglob("*.py"))) + base_allowed: Final = base_allowlist(args.base) if args.base else None + problems: Final = report(violations(paths), parse_allowlist(ALLOWLIST.read_text(encoding="utf-8")), base_allowed) + if problems: + print("\n".join(problems), file=sys.stderr) + return 1 + print("Layer imports: passed") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/layer_imports_allowlist.txt b/scripts/layer_imports_allowlist.txt new file mode 100644 index 00000000000..3bd69ce4adb --- /dev/null +++ b/scripts/layer_imports_allowlist.txt @@ -0,0 +1,149 @@ +# Known layer violations checked by scripts/check_layer_imports.py, one per line as +# This list may only shrink. Delete a line in the PR that removes the import, never add one +L1 function litellm/_service_logger.py litellm.proxy.proxy_server +L1 function litellm/batches/batch_utils.py litellm.proxy.openai_files_endpoints.common_utils +L1 function litellm/caching/qdrant_semantic_cache.py litellm.proxy.proxy_server +L1 function litellm/caching/redis_semantic_cache.py litellm.proxy.proxy_server +L1 function litellm/cost_calculator.py litellm.proxy._experimental.mcp_server.cost_calculator +L1 function litellm/experimental_mcp_client/client.py litellm.proxy._experimental.mcp_server.contracts +L1 function litellm/experimental_mcp_client/client.py litellm.proxy._experimental.mcp_server.interactions +L1 function litellm/google_genai/streaming_iterator.py litellm.proxy.pass_through_endpoints.streaming_handler +L1 function litellm/integrations/SlackAlerting/hanging_request_check.py litellm.proxy.proxy_server +L1 function litellm/integrations/SlackAlerting/slack_alerting.py litellm.proxy.common_utils.model_deprecation +L1 function litellm/integrations/SlackAlerting/slack_alerting.py litellm.proxy.management_helpers.user_invitation +L1 function litellm/integrations/SlackAlerting/slack_alerting.py litellm.proxy.proxy_server +L1 function litellm/integrations/SlackAlerting/slack_alerting.py litellm.proxy.spend_tracking.spend_management_endpoints +L1 function litellm/integrations/SlackAlerting/slack_alerting.py litellm.proxy.utils +L1 function litellm/integrations/azure_storage/azure_storage.py litellm.proxy.proxy_server +L1 function litellm/integrations/cloudzero/cloudzero.py litellm.proxy.proxy_server +L1 function litellm/integrations/cloudzero/database.py litellm.proxy.proxy_server +L1 function litellm/integrations/cloudzero/database.py litellm.proxy.spend_tracking.key_metadata_recovery +L1 function litellm/integrations/custom_guardrail.py litellm.proxy.guardrails.auto_router_compression +L1 function litellm/integrations/custom_guardrail.py litellm.proxy.guardrails.exception_utils +L1 function litellm/integrations/custom_guardrail.py litellm.proxy.proxy_server +L1 function litellm/integrations/custom_guardrail.py litellm.proxy.utils +L1 function litellm/integrations/custom_logger.py litellm.proxy._types +L1 function litellm/integrations/custom_sso_handler.py litellm.proxy.auth.trusted_proxy_utils +L1 function litellm/integrations/custom_sso_handler.py litellm.proxy.proxy_server +L1 function litellm/integrations/datadog/datadog.py litellm.proxy.litellm_pre_call_utils +L1 function litellm/integrations/email_alerting.py litellm.proxy.proxy_server +L1 function litellm/integrations/email_alerting.py litellm.proxy.utils +L1 function litellm/integrations/focus/database.py litellm.proxy.proxy_server +L1 function litellm/integrations/focus/database.py litellm.proxy.spend_tracking.key_metadata_recovery +L1 function litellm/integrations/focus/focus_logger.py litellm.proxy.proxy_server +L1 function litellm/integrations/gcs_bucket/gcs_bucket.py litellm.proxy.proxy_server +L1 function litellm/integrations/gcs_pubsub/pub_sub.py litellm.proxy.spend_tracking.spend_tracking_utils +L1 function litellm/integrations/gcs_pubsub/pub_sub.py litellm.proxy.utils +L1 function litellm/integrations/mavvrik_focus/mavvrik_focus_logger.py litellm.proxy.proxy_server +L1 function litellm/integrations/opentelemetry.py litellm.proxy.proxy_server +L1 function litellm/integrations/otel/logger.py litellm.proxy.proxy_server +L1 function litellm/integrations/prometheus.py litellm.proxy.auth.auth_checks +L1 function litellm/integrations/prometheus.py litellm.proxy.common_utils.callback_utils +L1 function litellm/integrations/prometheus.py litellm.proxy.common_utils.user_api_key_cache +L1 function litellm/integrations/prometheus.py litellm.proxy.litellm_pre_call_utils +L1 function litellm/integrations/prometheus.py litellm.proxy.management_endpoints.key_management_endpoints +L1 function litellm/integrations/prometheus.py litellm.proxy.management_endpoints.team_endpoints +L1 function litellm/integrations/prometheus.py litellm.proxy.proxy_server +L1 function litellm/integrations/prometheus.py litellm.proxy.utils +L1 function litellm/integrations/rubrik.py litellm.proxy.litellm_pre_call_utils +L1 function litellm/integrations/shadow_eval_logger.py litellm.proxy.auth.auth_checks +L1 function litellm/integrations/shadow_eval_logger.py litellm.proxy.db.shadow_eval_funnel +L1 function litellm/integrations/shadow_eval_logger.py litellm.proxy.proxy_server +L1 function litellm/integrations/vantage/vantage_logger.py litellm.proxy.proxy_server +L1 function litellm/integrations/vector_store_integrations/vector_store_pre_call_hook.py litellm.proxy.proxy_server +L1 function litellm/integrations/websearch_interception/handler.py litellm.proxy.auth.auth_checks +L1 function litellm/integrations/websearch_interception/handler.py litellm.proxy.litellm_pre_call_utils +L1 function litellm/integrations/websearch_interception/handler.py litellm.proxy.proxy_server +L1 function litellm/interactions/background_cost_polling.py litellm.proxy.spend_tracking.budget_reservation +L1 function litellm/litellm_core_utils/health_check_helpers.py litellm.proxy.litellm_pre_call_utils +L1 function litellm/litellm_core_utils/litellm_logging.py litellm.proxy.hooks.autorouter_baseline_cache +L1 function litellm/litellm_core_utils/litellm_logging.py litellm.proxy.hooks.dynamic_rate_limiter +L1 function litellm/litellm_core_utils/litellm_logging.py litellm.proxy.hooks.dynamic_rate_limiter_v3 +L1 function litellm/litellm_core_utils/litellm_logging.py litellm.proxy.spend_tracking.savings +L1 function litellm/litellm_core_utils/llm_judge.py litellm.proxy.proxy_server +L1 function litellm/litellm_core_utils/prompt_templates/common_utils.py litellm.proxy.openai_files_endpoints.common_utils +L1 function litellm/llms/__init__.py litellm.proxy._experimental.mcp_server.guardrail_translation +L1 function litellm/llms/a2a/chat/transformation.py litellm.proxy.agent_endpoints.agent_registry +L1 function litellm/llms/anthropic/chat/guardrail_translation/handler.py litellm.proxy.common_request_processing +L1 function litellm/llms/anthropic/chat/guardrail_translation/handler.py litellm.proxy.guardrails.anthropic_sse +L1 function litellm/llms/anthropic/chat/guardrail_translation/handler.py litellm.proxy.policy_engine.pipeline_executor +L1 function litellm/llms/anthropic/pass_through/adapters/handler.py litellm.proxy.proxy_server +L1 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy._types +L1 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy.auth.auth_checks +L1 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy.hooks.parallel_request_limiter_v3 +L1 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy.proxy_server +L1 function litellm/llms/anthropic/pass_through/messages/interceptors/advisor.py litellm.proxy.proxy_server +L1 function litellm/llms/anthropic/pass_through/messages/streaming_iterator.py litellm.proxy.pass_through_endpoints.streaming_handler +L1 function litellm/llms/azure/passthrough/transformation.py litellm.proxy.pass_through_endpoints.llm_provider_handlers.openai_passthrough_logging_handler +L1 function litellm/llms/litellm_proxy/skills/handler.py litellm.proxy.proxy_server +L1 function litellm/llms/openai/chat/guardrail_translation/handler.py litellm.proxy.common_request_processing +L1 function litellm/llms/openai/chat/guardrail_translation/handler.py litellm.proxy.policy_engine.pipeline_executor +L1 function litellm/llms/openai/responses/guardrail_translation/handler.py litellm.proxy.common_request_processing +L1 function litellm/llms/openai/responses/guardrail_translation/handler.py litellm.proxy.policy_engine.pipeline_executor +L1 function litellm/llms/pass_through/guardrail_translation/handler.py litellm.proxy.pass_through_endpoints.jsonpath_extractor +L1 function litellm/llms/pass_through/guardrail_translation/handler.py litellm.proxy.pass_through_endpoints.passthrough_guardrails +L1 function litellm/models/mcp_server.py litellm.proxy.common_utils.encrypt_decrypt_utils +L1 function litellm/repositories/credentials_repository.py litellm.proxy.common_utils.config_sync_pubsub +L1 function litellm/repositories/daily_activity_repository.py litellm.proxy.db.db_span +L1 function litellm/repositories/daily_activity_repository.py litellm.proxy.db.prisma_query_span +L1 function litellm/repositories/model_repository.py litellm.proxy.common_utils.encrypt_decrypt_utils +L1 function litellm/repositories/table_repositories.py litellm.proxy.common_utils.config_sync_pubsub +L1 function litellm/responses/litellm_completion_transformation/session_handler.py litellm.proxy.proxy_server +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy._experimental.mcp_server.mcp_server_manager +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy._experimental.mcp_server.server +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy._experimental.mcp_server.ui_session_utils +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy._types +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy.litellm_pre_call_utils +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy.management_endpoints.common_utils +L1 function litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy.proxy_server +L1 function litellm/responses/mcp/mcp_streaming_iterator.py litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp +L1 function litellm/responses/utils.py litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp +L1 function litellm/router.py litellm.proxy.auth.auto_router_checks +L1 function litellm/router.py litellm.proxy.guardrails.auto_router_compression +L1 function litellm/router.py litellm.proxy.pass_through_endpoints.llm_passthrough_endpoints +L1 function litellm/router_strategy/budget_limiter.py litellm.proxy.proxy_server +L1 function litellm/router_strategy/complexity_router/complexity_router.py litellm.proxy.common_utils.cache_aware_routing +L1 function litellm/rust_bridge/callbacks_legacy_python.py litellm.proxy.pass_through_endpoints.streaming_handler +L1 function litellm/secret_managers/cyberark_secret_manager.py litellm.proxy.proxy_server +L1 function litellm/secret_managers/google_secret_manager.py litellm.proxy.proxy_server +L1 function litellm/secret_managers/hashicorp_secret_manager.py litellm.proxy.proxy_server +L1 function litellm/types/roi_calculator.py litellm.proxy.roi_calculator.analytics +L1 module litellm/experimental_mcp_client/client.py litellm.proxy._experimental.mcp_server.mcp_debug +L1 module litellm/experimental_mcp_client/client.py litellm.proxy._experimental.mcp_server.result_conversion +L1 module litellm/google_genai/streaming_iterator.py litellm.proxy.pass_through_endpoints.success_handler +L1 module litellm/litellm_core_utils/custom_logger_registry.py litellm.proxy.hooks.dynamic_rate_limiter +L1 module litellm/litellm_core_utils/custom_logger_registry.py litellm.proxy.hooks.dynamic_rate_limiter_v3 +L1 module litellm/llms/anthropic/pass_through/messages/streaming_iterator.py litellm.proxy.pass_through_endpoints.success_handler +L1 module litellm/llms/litellm_proxy/skills/skill_search.py litellm.proxy.common_utils.semantic_text_index +L1 module litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy._experimental.mcp_server.catalog +L1 module litellm/responses/mcp/litellm_proxy_mcp_handler.py litellm.proxy._experimental.mcp_server.utils +L1 module litellm/types/proxy/management_endpoints/team_endpoints.py litellm.proxy.common_utils.timezone_utils +L2 function litellm/llms/__init__.py litellm.proxy._experimental.mcp_server.guardrail_translation +L2 function litellm/llms/a2a/chat/transformation.py litellm.proxy.agent_endpoints.agent_registry +L2 function litellm/llms/anthropic/chat/guardrail_translation/handler.py litellm.proxy.common_request_processing +L2 function litellm/llms/anthropic/chat/guardrail_translation/handler.py litellm.proxy.guardrails.anthropic_sse +L2 function litellm/llms/anthropic/chat/guardrail_translation/handler.py litellm.proxy.policy_engine.pipeline_executor +L2 function litellm/llms/anthropic/pass_through/adapters/handler.py litellm.proxy.proxy_server +L2 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy._types +L2 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy.auth.auth_checks +L2 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy.hooks.parallel_request_limiter_v3 +L2 function litellm/llms/anthropic/pass_through/context_management/editors/compact.py litellm.proxy.proxy_server +L2 function litellm/llms/anthropic/pass_through/messages/interceptors/advisor.py litellm.proxy.proxy_server +L2 function litellm/llms/anthropic/pass_through/messages/streaming_iterator.py litellm.proxy.pass_through_endpoints.streaming_handler +L2 function litellm/llms/azure/passthrough/transformation.py litellm.proxy.pass_through_endpoints.llm_provider_handlers.openai_passthrough_logging_handler +L2 function litellm/llms/litellm_proxy/skills/handler.py litellm.proxy.proxy_server +L2 function litellm/llms/openai/chat/guardrail_translation/handler.py litellm.proxy.common_request_processing +L2 function litellm/llms/openai/chat/guardrail_translation/handler.py litellm.proxy.policy_engine.pipeline_executor +L2 function litellm/llms/openai/responses/guardrail_translation/handler.py litellm.proxy.common_request_processing +L2 function litellm/llms/openai/responses/guardrail_translation/handler.py litellm.proxy.policy_engine.pipeline_executor +L2 function litellm/llms/pass_through/guardrail_translation/handler.py litellm.proxy.pass_through_endpoints.jsonpath_extractor +L2 function litellm/llms/pass_through/guardrail_translation/handler.py litellm.proxy.pass_through_endpoints.passthrough_guardrails +L2 module litellm/llms/anthropic/pass_through/messages/streaming_iterator.py litellm.proxy.pass_through_endpoints.success_handler +L2 module litellm/llms/litellm_proxy/skills/skill_search.py litellm.proxy.common_utils.semantic_text_index +L3 function litellm/types/integrations/prometheus.py litellm +L3 function litellm/types/roi_calculator.py litellm.proxy.roi_calculator.analytics +L3 function litellm/types/router.py litellm.secret_managers.main +L3 module litellm/types/proxy/management_endpoints/team_endpoints.py litellm.proxy.common_utils.timezone_utils +LEAF function litellm/litellm_core_utils/core_helpers.py litellm +LEAF function litellm/litellm_core_utils/core_helpers.py litellm.exceptions +LEAF function litellm/litellm_core_utils/core_helpers.py litellm.llms.base_llm.chat.transformation diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index edb69f24e29..e3eaecca4c4 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -106,6 +106,9 @@ litellm_py_files=$(scope_match "$litellm_py_pattern") if [ -n "$(scope_match '^(litellm/proxy/_experimental/mcp_server/|scripts/check_mcp_operation_boundary\.py)')" ]; then uv run --no-sync python scripts/check_mcp_operation_boundary.py || exit 1 fi +if [ -n "$(scope_match '^(litellm/.*\.py|scripts/check_layer_imports\.py|scripts/layer_imports_allowlist\.txt)$')" ]; then + uv run --no-sync python scripts/check_layer_imports.py ${base_ref:+--base "$base_ref"} || exit 1 +fi e2e_py_files=$(scope_match "$e2e_py_pattern") test_tree_files=$(scope_match "$test_tree_pattern") # ruff format (and CI's format step) skip enterprise; the rest of make lint covers it. diff --git a/tests/unit/test_check_layer_imports.py b/tests/unit/test_check_layer_imports.py new file mode 100644 index 00000000000..752ef1a6ba9 --- /dev/null +++ b/tests/unit/test_check_layer_imports.py @@ -0,0 +1,248 @@ +from pathlib import Path + +import pytest + +from scripts.check_layer_imports import git_base_allowlist, main + + +def build_tree(root: Path, files: dict[str, str], allowlist: str = "") -> None: + for relative, source in { + "litellm/proxy/_types.py": "", + "litellm/main.py": "", + "litellm/router.py": "", + **files, + }.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(source) + (root / "scripts").mkdir(exist_ok=True) + (root / "scripts/layer_imports_allowlist.txt").write_text(allowlist) + + +def run_check( + root: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + base_allowed: frozenset[str] | None = None, +) -> tuple[int, str]: + monkeypatch.chdir(root) + code = main(["--base", "origin/main"], lambda _ref: base_allowed) + return code, capsys.readouterr().err + + +@pytest.mark.parametrize( + ("path", "source", "expected"), + ( + ( + "litellm/integrations/foo.py", + "from litellm.proxy._types import UserAPIKeyAuth\n", + "litellm/integrations/foo.py:1: L1 forbids litellm.integrations.foo importing litellm.proxy._types (module level)", + ), + ( + "litellm/integrations/foo.py", + "def f():\n from litellm import proxy\n", + "litellm/integrations/foo.py:2: L1 forbids litellm.integrations.foo importing litellm.proxy (function level)", + ), + ( + "litellm/llms/foo/handler.py", + "import os\nfrom litellm import main\n", + "litellm/llms/foo/handler.py:2: L2 forbids litellm.llms.foo.handler importing litellm.main (module level)", + ), + ( + "litellm/llms/foo/handler.py", + "import litellm.router\n", + "litellm/llms/foo/handler.py:1: L2 forbids litellm.llms.foo.handler importing litellm.router (module level)", + ), + ( + "litellm/types/foo.py", + "from litellm.integrations.custom_logger import CustomLogger\n", + "litellm/types/foo.py:1: L3 forbids litellm.types.foo importing litellm.integrations.custom_logger (module level)", + ), + ( + "litellm/types/foo.py", + "import litellm\n", + "litellm/types/foo.py:1: L3 forbids litellm.types.foo importing litellm (module level)", + ), + ( + "litellm/types/sub/foo.py", + "from ...proxy import _types\n", + "litellm/types/sub/foo.py:1: L3 forbids litellm.types.sub.foo importing litellm.proxy._types (module level)", + ), + ( + "litellm/litellm_core_utils/duration_parser.py", + "def f():\n from litellm.utils import x\n", + "litellm/litellm_core_utils/duration_parser.py:2: LEAF forbids litellm.litellm_core_utils.duration_parser " + "importing litellm.utils (function level)", + ), + ( + "litellm/litellm_core_utils/llm_response_utils/get_headers.py", + "from litellm.router_strategy import x\n", + "litellm/litellm_core_utils/llm_response_utils/get_headers.py:1: LEAF forbids " + "litellm.litellm_core_utils.llm_response_utils.get_headers importing litellm.router_strategy (module level)", + ), + ( + "litellm/integrations/foo.py", + "from typing import TYPE_CHECKING\nif TYPE_CHECKING:\n pass\nelse:\n import litellm.proxy.utils\n", + "litellm/integrations/foo.py:5: L1 forbids litellm.integrations.foo importing litellm.proxy.utils (module level)", + ), + ), +) +def test_new_layer_violation_fails( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + path: str, + source: str, + expected: str, +) -> None: + build_tree(tmp_path, {path: source}) + code, err = run_check(tmp_path, monkeypatch, capsys) + assert code == 1 + assert expected in err.splitlines() + + +@pytest.mark.parametrize( + ("path", "source"), + ( + ( + "litellm/integrations/foo.py", + "from typing import TYPE_CHECKING\nif TYPE_CHECKING:\n from litellm.proxy import _types\n", + ), + ("litellm/integrations/foo.py", "import typing\nif typing.TYPE_CHECKING:\n import litellm.proxy.utils\n"), + ( + "litellm/integrations/foo.py", + "def f():\n if TYPE_CHECKING:\n from litellm.proxy._types import X\n", + ), + ("litellm/proxy/foo.py", "from litellm.proxy._types import X\nimport litellm.main\n"), + ("litellm/llms/foo.py", "import litellm\nfrom litellm import Router\nfrom litellm.router_strategy import x\n"), + ( + "litellm/types/foo.py", + "from litellm.types.utils import X\nfrom litellm.models.team import Y\nfrom litellm.constants import Z\n" + "from litellm._logging import verbose_logger\nfrom litellm._uuid import uuid\nimport pydantic\n" + "from litellm.litellm_core_utils.completion_timeout import CompletionTimeout\n" + "from litellm.litellm_core_utils.core_helpers import normalize_drop_params\n" + "from litellm.litellm_core_utils.provider_affinity import validate_provider_affinity_header_name\n" + "from litellm.litellm_core_utils.duration_parser import duration_in_seconds\n", + ), + ( + "litellm/litellm_core_utils/core_helpers.py", + "from litellm.litellm_core_utils.llm_response_utils.get_headers import get_response_headers\n" + "from litellm.types.utils import X\nfrom litellm._logging import verbose_logger\n", + ), + ("litellm/litellm_core_utils/other.py", "import litellm.router\nfrom litellm.utils import x\n"), + ), +) +def test_allowed_imports_pass( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], path: str, source: str +) -> None: + build_tree(tmp_path, {path: source}) + assert run_check(tmp_path, monkeypatch, capsys) == (0, "") + + +def test_allowlisted_violation_passes_and_stale_entry_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + llms_source = "from litellm.proxy._types import A\nfrom litellm.proxy._types import B\n" + entries = ( + "# header\n\nL1 module litellm/llms/foo.py litellm.proxy._types\n" + "L2 module litellm/llms/foo.py litellm.proxy._types\n" + ) + build_tree(tmp_path, {"litellm/llms/foo.py": llms_source}, entries) + assert run_check(tmp_path, monkeypatch, capsys) == (0, "") + + build_tree(tmp_path, {"litellm/llms/foo.py": "def f():\n from litellm.proxy._types import A\n"}, entries) + code, err = run_check(tmp_path, monkeypatch, capsys) + assert code == 1 + assert sorted(err.splitlines()) == [ + "litellm/llms/foo.py:2: L1 forbids litellm.llms.foo importing litellm.proxy._types (function level)", + "litellm/llms/foo.py:2: L2 forbids litellm.llms.foo importing litellm.proxy._types (function level)", + "scripts/layer_imports_allowlist.txt: stale entry 'L1 module litellm/llms/foo.py litellm.proxy._types' " + "no longer matches an import, delete it", + "scripts/layer_imports_allowlist.txt: stale entry 'L2 module litellm/llms/foo.py litellm.proxy._types' " + "no longer matches an import, delete it", + ] + + +def test_allowlist_entry_added_since_base_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + build_tree( + tmp_path, + { + "litellm/types/foo.py": "from litellm.router import Router\n", + "litellm/types/bar.py": "import litellm.main\n", + }, + "L3 module litellm/types/foo.py litellm.router\nL3 module litellm/types/bar.py litellm.main\n", + ) + base = frozenset({"L3 module litellm/types/foo.py litellm.router"}) + + assert run_check(tmp_path, monkeypatch, capsys, base) == ( + 1, + "scripts/layer_imports_allowlist.txt: entry 'L3 module litellm/types/bar.py litellm.main' is not in the " + "base allowlist, fix the import instead of allowlisting it\n", + ) + assert run_check(tmp_path, monkeypatch, capsys, base | {"L3 module litellm/types/bar.py litellm.main"}) == (0, "") + + +def test_module_import_moved_into_a_function_keeps_its_base_entry( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + build_tree( + tmp_path, + {"litellm/types/foo.py": "def f():\n from litellm.router import Router\n"}, + "L3 function litellm/types/foo.py litellm.router\n", + ) + + assert run_check(tmp_path, monkeypatch, capsys, frozenset({"L3 module litellm/types/foo.py litellm.router"})) == ( + 0, + "", + ) + + +def test_function_import_moved_to_module_level_is_not_in_the_base( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + build_tree( + tmp_path, + {"litellm/types/foo.py": "from litellm.router import Router\n"}, + "L3 module litellm/types/foo.py litellm.router\n", + ) + + assert run_check(tmp_path, monkeypatch, capsys, frozenset({"L3 function litellm/types/foo.py litellm.router"})) == ( + 1, + "scripts/layer_imports_allowlist.txt: entry 'L3 module litellm/types/foo.py litellm.router' is not in the " + "base allowlist, fix the import instead of allowlisting it\n", + ) + + +def test_missing_base_allowlist_skips_growth_check( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + build_tree( + tmp_path, + {"litellm/types/foo.py": "from litellm.router import Router\n"}, + "L3 module litellm/types/foo.py litellm.router\n", + ) + + assert run_check(tmp_path, monkeypatch, capsys, None) == (0, "") + + +def test_base_allowlist_is_read_at_the_merge_base_not_the_branch_tip() -> None: + allowlist = "scripts/layer_imports_allowlist.txt" + outputs = { + ("merge-base", "origin/main", "HEAD"): "forkpoint\n", + ("ls-tree", "--name-only", "forkpoint", "--", allowlist): f"{allowlist}\n", + ("show", f"forkpoint:{allowlist}"): "L3 module litellm/types/foo.py litellm.router\n", + ("ls-tree", "--name-only", "origin/main", "--", allowlist): f"{allowlist}\n", + ("show", f"origin/main:{allowlist}"): "", + } + + assert git_base_allowlist("origin/main", lambda args: outputs.get(tuple(args), "")) == frozenset( + {"L3 module litellm/types/foo.py litellm.router"} + ) + + +def test_base_without_an_allowlist_skips_the_growth_check() -> None: + outputs = {("merge-base", "origin/main", "HEAD"): "forkpoint\n"} + + assert git_base_allowlist("origin/main", lambda args: outputs.get(tuple(args), "")) is None