diff --git a/AGENTS.md b/AGENTS.md index 7bea7a56cbc..5c0b2084db2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,7 +101,7 @@ Follow these coding conventions for new/updated code (a three-line fix in a lega - Never-nester: early returns over deep nesting - Don't throw. Model failures as values (One function (e.g., raise_public) maps error union to existing public exception contracts via exhaustive match + assert_never) - No mutation. Don't reassign variables, global or local. Instead of mutable lists and dicts, prefer tuples, frozen dataclasses (with slots=True), `MappingProxyType`, etc. - - Annotate every variable with `: Final` (LIT010). A `TypeVar`, `ParamSpec`, or `TypeVarTuple` is the exception and stays bare, since mypy reads a Final one as `Any`. Unpacking and walrus targets cannot carry the annotation, so they are implicitly final. Don't rebind them. Never rebind or mutate function parameters (LIT011). `self`/`cls` attribute stores are the exception. If rebinding or in-place mutation is truly unavoidable, suppress with `# rebind-ok: ` + - Annotate every variable with `: Final` (LIT010). A `TypeVar`, `ParamSpec`, `TypeVarTuple`, or `TypeAliasType` is the exception and stays bare, since mypy reads a Final one as a plain variable instead of a type. Unpacking and walrus targets cannot carry the annotation, so they are implicitly final. Don't rebind them. Never rebind or mutate function parameters (LIT011). `self`/`cls` attribute stores are the exception. If rebinding or in-place mutation is truly unavoidable, suppress with `# rebind-ok: ` - Qualify every TypedDict field with `ReadOnly[...]` (LIT012), which nests freely with `Required` / `NotRequired` / `Annotated` in any order. If making the key writable is truly unavoidable, suppress with `# writable-ok: ` - Comprehensions take at most one `for` clause and one `if` clause (LIT014). Split stacked clauses into a helper generator, a named intermediate, or a plain loop. Suppress with `# comprehension-ok: ` only when unavoidable - Every pydantic model must be frozen (LIT015), set via `model_config = ConfigDict(frozen=True)`, a dict-literal `model_config`, an inner `class Config`, or the class keywords. Subclasses inherit it unless they override it. Replace in-place field writes with `model_copy(update=...)`. If making the model mutable is truly unavoidable, suppress with `# frozen-ok: ` on the `class` line diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 9e4da1edc9f..254f52321dc 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -54,11 +54,11 @@ LIT010 Variable assignment without a `Final` declaration. Every local and modul that namespace is the SDK's runtime-settable config surface (users follow the documented `litellm.api_key = ...` pattern and the proxy rebinds these via setattr), and the package ships py.typed, so a Final there turns every - documented downstream assignment into a mypy error. Type variables are the - reverse: a `TypeVar`, `ParamSpec`, or `TypeVarTuple` stays bare, and a Final one - trips, since mypy reads it as a plain variable and every signature using it as - Any. Suppress a deliberately rebindable name with `# rebind-ok: ` on - each offending line. + documented downstream assignment into a mypy error. Type variables and type + aliases are the reverse: a `TypeVar`, `ParamSpec`, `TypeVarTuple`, or + `TypeAliasType` stays bare, and a Final one trips, since mypy reads it as a + plain variable instead of a type. Suppress a deliberately rebindable name + with `# rebind-ok: ` on each offending line. LIT011 Function-argument mutation: a parameter that is re-bound (`param = ...`, `param += ...`, a `for`/`with`/unpacking/walrus target, `del param`, or a re-bind in a nested function under `nonlocal param`) or mutated in place diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 95b937fd145..1c6bb903b72 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -15,7 +15,7 @@ # -> `make lint-e2e-basedpyright` (test-linting.yml's e2e type-check step) # - tests/e2e and tests/e2e_harness Python # -> raw HTTP client ban (test-linting.yml's check_e2e_no_raw_requests) -# - litellm/router.py, uv.lock +# - litellm/router.py, litellm/__init__.py, uv.lock # -> mypy Router stub guard (test-linting.yml's check_router_constructor_stub) # - tests/ Python, ruff-tests.toml, scripts/check_test_quality.py, # scripts/test_quality_gate.py @@ -112,7 +112,7 @@ litellm_py_files=$(scope_match "$litellm_py_pattern") if [ -n "$(scope_match '^(litellm/proxy/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/router\.py|tests/code_coverage_tests/check_router_constructor_stub\.py|uv\.lock)$')" ]; then +if [ -n "$(scope_match '^(litellm/(router|__init__)\.py|tests/code_coverage_tests/check_router_constructor_stub\.py|uv\.lock)$')" ]; then uv run --no-sync python tests/code_coverage_tests/check_router_constructor_stub.py || exit 1 fi e2e_py_files=$(scope_match "$e2e_py_pattern") diff --git a/tests/code_coverage_tests/check_router_constructor_stub.py b/tests/code_coverage_tests/check_router_constructor_stub.py index 8ab8b82e695..eb69f7b527b 100644 --- a/tests/code_coverage_tests/check_router_constructor_stub.py +++ b/tests/code_coverage_tests/check_router_constructor_stub.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Fail unless mypy reads the Router constructor stub in litellm/router.py. +"""Fail unless mypy reads the Router constructor stub in litellm/router.py through `from litellm import Router`. litellm ships py.typed, so a stub that mypy reads as Any accepts every argument name in SDK users' code, while basedpyright, which `make lint-e2e-basedpyright` runs, can still read it. @@ -12,7 +12,7 @@ from pathlib import Path from typing import Final PROBE: Final = """\ -from litellm.router import Router +from litellm import Router Router(model_list=[], num_retries=2) Router(model_list=[], redis_host="localhost", redis_port="6379") @@ -31,6 +31,7 @@ def mypy_errors(probe: Path) -> tuple[str, ...]: "--config-file=", "--follow-imports=skip", "--ignore-missing-imports", + "litellm/__init__.py", "litellm/router.py", str(probe), ), diff --git a/tests/unit/test_pre_commit_lint.py b/tests/unit/test_pre_commit_lint.py index 080a11188b7..72ec3707a64 100644 --- a/tests/unit/test_pre_commit_lint.py +++ b/tests/unit/test_pre_commit_lint.py @@ -454,7 +454,9 @@ def test_an_e2e_type_check_input_alone_runs_the_e2e_basedpyright_step(tmp_path: assert "ran: tests/e2e basedpyright" in proc.stdout -@pytest.mark.parametrize(("changed", "runs"), (("litellm/router.py", True), ("litellm/foo.py", False))) +@pytest.mark.parametrize( + ("changed", "runs"), (("litellm/router.py", True), ("litellm/__init__.py", True), ("litellm/foo.py", False)) +) def test_the_router_stub_guard_runs_only_when_the_router_changes(tmp_path: Path, changed: str, runs: bool) -> None: repo, bin_dir = _sandbox(tmp_path) _commit_all(repo, "base")