From dcb632498699175e6538e8f2d3f353b4df6b6375 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 7 Nov 2026 06:46:16 -0800 Subject: [PATCH] fix(lint): run the Router stub guard and e2e type check from make check make check now runs check_router_constructor_stub.py when litellm/router.py or uv.lock changes, and the e2e basedpyright step on pyproject.toml, uv.lock, pyrightconfig.json, and litellm-proxy-extras Python, matching CI. CI's e2e type check also fires on litellm-proxy-extras Python, which tests/e2e/perf imports. LIT010 treats TypeAliasType like a type variable, since mypy also rejects a Final type alias. --- .github/workflows/test-linting.yml | 4 +- scripts/check_type_discipline.py | 6 +-- scripts/pre_commit_lint.sh | 24 +++++++--- tests/e2e/AGENTS.md | 2 +- tests/e2e/CONTRIBUTING.md | 2 +- tests/unit/test_check_type_discipline.py | 9 +++- tests/unit/test_lint_workflow_diff_gates.py | 3 ++ tests/unit/test_pre_commit_lint.py | 49 ++++++++++++++++++++- 8 files changed, 84 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test-linting.yml b/.github/workflows/test-linting.yml index 2088b6f6af7..1ad8326ba47 100644 --- a/.github/workflows/test-linting.yml +++ b/.github/workflows/test-linting.yml @@ -191,10 +191,10 @@ jobs: - name: Check tests/e2e basedpyright (zero errors) if: steps.changes.outputs.decision != 'skip' run: | - if git diff --name-only --diff-filter=ACMRD "$GATE_BASE_SHA" HEAD -- ':(glob)tests/e2e/**/*.py' ':(glob)tests/e2e_harness/**/*.py' ':(glob)litellm/**/*.py' pyrightconfig.json pyproject.toml uv.lock | grep -q .; then + if git diff --name-only --diff-filter=ACMRD "$GATE_BASE_SHA" HEAD -- ':(glob)tests/e2e/**/*.py' ':(glob)tests/e2e_harness/**/*.py' ':(glob)litellm/**/*.py' ':(glob)litellm-proxy-extras/**/*.py' pyrightconfig.json pyproject.toml uv.lock | grep -q .; then uv run --no-sync basedpyright tests/e2e tests/e2e_harness else - echo "No changed tests/e2e, litellm Python, or dependency files; skipping." + echo "No changed tests/e2e, litellm, or litellm-proxy-extras Python, or dependency files; skipping." fi - name: Run the e2e harness tests diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index ea86c19b4de..9e4da1edc9f 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -461,7 +461,7 @@ def iter_guard_violations(path: Path, tree: ast.AST) -> Iterator[Violation]: # --------------------------------------------------------------------------- # CONSTANT_DECLARATIONS = frozenset(("Final", "TypeAlias")) -TYPE_VARIABLE_FACTORIES = frozenset(("TypeVar", "ParamSpec", "TypeVarTuple")) +TYPE_VARIABLE_FACTORIES = frozenset(("TypeVar", "ParamSpec", "TypeVarTuple", "TypeAliasType")) CONFIG_SURFACE_PARTS = ("litellm", "__init__.py") SELF_PARAMS = frozenset(("self", "cls")) NESTED_SCOPES = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Lambda) @@ -697,8 +697,8 @@ def iter_final_type_variable_violations(path: Path, tree: ast.AST) -> Iterator[V path, line, "LIT010", - f"`{name}` is a type variable declared Final: mypy then reads it as a plain " - f"variable and every signature using it as Any, so remove the Final", + f"`{name}` is a type variable or type alias declared Final: mypy then reads it as a plain " + f"variable instead of a type, so remove the Final", ) case _: pass diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 95b5942fa8b..95b937fd145 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -10,9 +10,13 @@ # with origin's current default branch, untracked files included # The per-area checks: # - litellm/ Python -> `make lint` (test-linting.yml's python job) -# - tests/e2e and tests/e2e_harness Python +# - tests/e2e, tests/e2e_harness, and litellm-proxy-extras Python, pyrightconfig.json, +# pyproject.toml, uv.lock # -> `make lint-e2e-basedpyright` (test-linting.yml's e2e type-check step) -# + raw HTTP client ban (test-linting.yml's check_e2e_no_raw_requests) +# - 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 +# -> 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 # -> ruff over ruff-tests.toml + `make lint-test-quality` (test-linting.yml's @@ -98,6 +102,7 @@ existing_files() { litellm_py_pattern='^litellm/.*\.py$' e2e_py_pattern='^tests/e2e(_harness)?/.*\.py$' +e2e_types_pattern='^(tests/e2e(_harness)?/.*\.py|litellm-proxy-extras/.*\.py|pyrightconfig\.json|pyproject\.toml|uv\.lock)$' test_tree_pattern='^(tests/.*\.py|ruff-tests\.toml|scripts/(check_test_quality|test_quality_gate)\.py)$' spec_pattern='^(litellm/.*|enterprise/.*|pyproject\.toml|uv\.lock|ui/litellm-dashboard/(scripts/gen-api-types\.mjs|package\.json|package-lock\.json|src/lib/http/schema\.d\.ts))$' ui_prettier_pattern='^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs|json|css|scss|md|mdx|yml|yaml|html)$' @@ -107,7 +112,11 @@ 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 + 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") +e2e_types_files=$(scope_match "$e2e_types_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. fmt_files=$(printf '%s\n' "$litellm_py_files" | grep -v '^litellm/enterprise/' | existing_files) @@ -144,7 +153,8 @@ if [ -n "$staged" ]; then printf '%s\n' "$missed" | sed 's/^/ /' >&2 } warn_skipped "Python lint (make lint)" "$litellm_py_pattern" "$litellm_py_files" - warn_skipped "tests/e2e checks (basedpyright + raw HTTP client ban)" "$e2e_py_pattern" "$e2e_py_files" + warn_skipped "tests/e2e basedpyright" "$e2e_types_pattern" "$e2e_types_files$litellm_py_files" + warn_skipped "tests/e2e raw HTTP client ban" "$e2e_py_pattern" "$e2e_py_files" warn_skipped "test-tree lint (ruff-tests.toml + test-quality gate)" "$test_tree_pattern" "$test_tree_files" warn_skipped "dashboard lint (prettier + eslint + lint budgets)" "$ui_prettier_pattern" "$ui_prettier_changed" warn_skipped "dashboard API-type sync (npm run gen:api)" "$spec_pattern" "$spec_files" @@ -224,7 +234,7 @@ if [ -n "$litellm_py_files" ]; then set +m fi -if [ -n "$e2e_py_files" ] && [ -z "$litellm_py_files" ]; then +if [ -n "$e2e_types_files" ] && [ -z "$litellm_py_files" ]; then echo "check: type-checking tests/e2e (make lint-e2e-basedpyright)" make lint-e2e-basedpyright || { echo "✗ tests/e2e basedpyright failed. Fix the errors above, then re-run make check." >&2; status=1; } fi @@ -331,13 +341,15 @@ summary_item() { echo "check: summary" summary_item "Python lint (make lint)" "$litellm_py_files" "no litellm/ Python files in scope" -summary_item "tests/e2e checks (basedpyright + raw HTTP client ban)" "$e2e_py_files" "no tests/e2e Python files in scope" +summary_item "tests/e2e basedpyright" "$e2e_types_files$litellm_py_files" \ + "no litellm/, tests/e2e, or litellm-proxy-extras Python, or type-check inputs in scope" +summary_item "tests/e2e raw HTTP client ban" "$e2e_py_files" "no tests/e2e Python files in scope" summary_item "test-tree lint (ruff-tests.toml + test-quality gate)" "$test_tree_files" \ "no tests/ Python files or test-tree lint inputs in scope" summary_item "dashboard lint (prettier + eslint + lint budgets)" "$ui_prettier_changed$ui_eslint_changed" "no dashboard files in scope" summary_item "dashboard API-type sync (npm run gen:api)" "$spec_files" "no litellm/, enterprise/, dependency, or generator files in scope" -if [ -z "$litellm_py_files$e2e_py_files$test_tree_files$ui_prettier_changed$ui_eslint_changed$spec_files" ]; then +if [ -z "$litellm_py_files$e2e_types_files$test_tree_files$ui_prettier_changed$ui_eslint_changed$spec_files" ]; then echo "check: NOTE - no gating lint check matches the files in scope, so nothing ran:" >&2 printf '%s\n' "$scope" | sed 's/^/ /' >&2 echo " A pass here is a no-op, not a lint verdict." >&2 diff --git a/tests/e2e/AGENTS.md b/tests/e2e/AGENTS.md index b4035045c8d..2efbdde33a3 100644 --- a/tests/e2e/AGENTS.md +++ b/tests/e2e/AGENTS.md @@ -131,7 +131,7 @@ Current limits: Bedrock cannot be mounted in record or replay (SigV4 signs the H ## Typing -The harness is fully typed with no error budget: `make lint-e2e-basedpyright` must report zero basedpyright errors, and CI enforces that on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `pyrightconfig.json`, `pyproject.toml` or `uv.lock`. When a response field is untyped, model it in `models.py` (just the fields you read) and let pydantic validate it, rather than threading a `dict` or `Any` through the test +The harness is fully typed with no error budget: `make lint-e2e-basedpyright` must report zero basedpyright errors, and CI enforces that on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `litellm-proxy-extras/**/*.py`, `pyrightconfig.json`, `pyproject.toml` or `uv.lock`. When a response field is untyped, model it in `models.py` (just the fields you read) and let pydantic validate it, rather than threading a `dict` or `Any` through the test ## Typed test metadata diff --git a/tests/e2e/CONTRIBUTING.md b/tests/e2e/CONTRIBUTING.md index 6d94db48861..9e964d55676 100644 --- a/tests/e2e/CONTRIBUTING.md +++ b/tests/e2e/CONTRIBUTING.md @@ -224,7 +224,7 @@ Mark live tests with `@pytest.mark.e2e` (on the class or the module). Pure cover Before you push -1. Run `make lint-e2e-basedpyright` (or `make check` with your changes staged); the harness is fully typed and the gate allows zero basedpyright errors, enforced in CI on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `pyrightconfig.json`, `pyproject.toml`, or `uv.lock` +1. Run `make lint-e2e-basedpyright` (or `make check` with your changes staged); the harness is fully typed and the gate allows zero basedpyright errors, enforced in CI on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `litellm-proxy-extras/**/*.py`, `pyrightconfig.json`, `pyproject.toml`, or `uv.lock` 2. Add the models your test needs to the config your local proxy loads diff --git a/tests/unit/test_check_type_discipline.py b/tests/unit/test_check_type_discipline.py index e4240d8c210..e7dbdb669af 100644 --- a/tests/unit/test_check_type_discipline.py +++ b/tests/unit/test_check_type_discipline.py @@ -305,7 +305,14 @@ def test_single_leading_underscore_is_not_exempt(tmp_path): @pytest.mark.parametrize( "declaration", - ("TypeVar('_T')", "ParamSpec('_T')", "TypeVarTuple('_T')", "typing.TypeVar('_T')", "typing_extensions.ParamSpec('_T')"), + ( + "TypeVar('_T')", + "ParamSpec('_T')", + "TypeVarTuple('_T')", + "TypeAliasType('_T', int)", + "typing.TypeVar('_T')", + "typing_extensions.ParamSpec('_T')", + ), ) def test_a_type_variable_stays_bare_and_a_final_one_trips(tmp_path, declaration): assert "LIT010" not in _codes(tmp_path, f"_T = {declaration}\n") diff --git a/tests/unit/test_lint_workflow_diff_gates.py b/tests/unit/test_lint_workflow_diff_gates.py index 5e6ae9d285a..fb6212ecd45 100644 --- a/tests/unit/test_lint_workflow_diff_gates.py +++ b/tests/unit/test_lint_workflow_diff_gates.py @@ -248,7 +248,10 @@ def test_diff_gate_selects_top_level_and_nested_python_files_only(tmp_path: Path ( "tests/e2e_harness/top_level_module.py", "tests/e2e_harness/pkg/sub/nested_module.py", + "litellm-proxy-extras/litellm_proxy_extras/pkg/nested_module.py", "pyrightconfig.json", + "pyproject.toml", + "uv.lock", ), ) def test_e2e_basedpyright_gate_also_fires_on_harness_python_and_pyrightconfig(tmp_path: Path, trigger: str) -> None: diff --git a/tests/unit/test_pre_commit_lint.py b/tests/unit/test_pre_commit_lint.py index ef84ab9fbc8..080a11188b7 100644 --- a/tests/unit/test_pre_commit_lint.py +++ b/tests/unit/test_pre_commit_lint.py @@ -85,6 +85,10 @@ case "$*" in [ -n "${STUB_ARGS_DIR:-}" ] && echo "$*" >> "$STUB_ARGS_DIR/ruff_tests.args" [ "${STUB_FAIL:-}" = "tests-ruff" ] && exit 1 ;; + "run --no-sync python tests/code_coverage_tests/check_router_constructor_stub.py") + [ -n "${STUB_ARGS_DIR:-}" ] && echo "$*" >> "$STUB_ARGS_DIR/router_stub.args" + [ "${STUB_FAIL:-}" = "router-stub" ] && exit 1 + ;; esac exit 0 """ @@ -421,12 +425,55 @@ def test_run_ends_with_a_summary_of_ran_and_skipped_blocks(tmp_path: Path) -> No assert "ran: Python lint (make lint)" in proc.stdout assert "ran: dashboard lint (prettier + eslint + lint budgets)" in proc.stdout assert "ran: dashboard API-type sync (npm run gen:api)" in proc.stdout - assert "skipped: tests/e2e checks (basedpyright + raw HTTP client ban) (no tests/e2e Python files in scope)" in proc.stdout + assert "ran: tests/e2e basedpyright" in proc.stdout + assert "skipped: tests/e2e raw HTTP client ban (no tests/e2e Python files in scope)" in proc.stdout assert TEST_TREE_SKIPPED in proc.stdout assert "check: PASS" in proc.stdout assert "check: FAIL" not in proc.stdout +@pytest.mark.parametrize( + "type_input", + ( + "tests/e2e_harness/collector.py", + "litellm-proxy-extras/litellm_proxy_extras/utils.py", + "pyrightconfig.json", + "pyproject.toml", + "uv.lock", + ), +) +def test_an_e2e_type_check_input_alone_runs_the_e2e_basedpyright_step(tmp_path: Path, type_input: str) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + args_dir = tmp_path / "args" + args_dir.mkdir() + _stage_file(repo, type_input, "x = 1\n") + proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "lint-e2e-basedpyright" in _recorded(args_dir, "make.args") + assert "ran: tests/e2e basedpyright" in proc.stdout + + +@pytest.mark.parametrize(("changed", "runs"), (("litellm/router.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") + args_dir = tmp_path / "args" + args_dir.mkdir() + _stage_file(repo, changed, "x = 3\n") + proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert (args_dir / "router_stub.args").exists() is runs + + +def test_a_failing_router_stub_guard_fails_the_run(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + _stage_file(repo, "litellm/router.py", "x = 3\n") + proc = _run(repo, bin_dir, {"STUB_FAIL": "router-stub"}) + assert proc.returncode == 1 + + @pytest.mark.parametrize( "spec_input", (