diff --git a/CLAUDE.md b/CLAUDE.md index 0354e3def53..abd5993eb1a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,7 +41,7 @@ Python max line length is 120, not 88 When you fix violations gated by `ruff-strict-budget.json`, `type-discipline-budget.json`, or `basedpyright-code-budget.json`, run `make lint-budget-update` and commit the lowered limits so the ceilings ratchet down instead of leaving stale headroom. It measures the working tree, so it must contain exactly the fixes you're committing -Run `make check` (formerly `make pre-commit`, which still works as an alias) before every commit, merge commits included. It runs the CI-gating lint scoped to your staged files, so stage everything you intend to commit first; it warns about changed files you left unstaged and names the checks that were skipped because of them. With nothing staged it instead checks the working tree's diff against the merge base with origin/litellm_internal_staging, which is how you predict the CI lint on an already-committed branch, e.g. right after a merge commit +Run `make check` (formerly `make pre-commit`, which still works as an alias) before every commit, merge commits included. It runs the CI-gating lint scoped to your staged files, so stage everything you intend to commit first; it warns about changed files you left unstaged and names the checks that were skipped because of them. With nothing staged it instead checks the working tree's diff against the merge base with origin/litellm_internal_staging, which is how you predict the CI lint on an already-committed branch, e.g. right after a merge commit. Deleted files count toward which checks run (a deletion alone can turn CI red) in both modes `make check` saves its complete output to a log file in .git (overwriting previous logs) and prints that path as its first and last output lines. To inspect a run, read or grep that log instead of re-running the multi-minute checks just to see a different slice diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 1bf7fe17832..afe55603466 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -44,7 +44,7 @@ fi repo_root=$(git rev-parse --show-toplevel) cd "$repo_root" -staged=$(git diff --cached --name-only --diff-filter=ACMR) +staged=$(git diff --cached --name-only --diff-filter=ACMRD) unstaged=$(git diff --name-only) untracked=$(git ls-files --others --exclude-standard) @@ -57,7 +57,7 @@ else echo " Fix: git fetch origin litellm_internal_staging" >&2 exit 1 } - scope=$(printf '%s\n' "$(git diff --name-only --diff-filter=ACMR "$merge_base")" "$untracked" | sed '/^$/d' | sort -u) + scope=$(printf '%s\n' "$(git diff --name-only --diff-filter=ACMRD "$merge_base")" "$untracked" | sed '/^$/d' | sort -u) if [ -z "$scope" ]; then echo "check: nothing to check (no staged files, no working-tree changes, no branch changes vs origin/litellm_internal_staging)" exit 0 @@ -68,6 +68,12 @@ fi scope_match() { printf '%s\n' "$scope" | grep -E "$1" || true; } +existing_files() { + while IFS= read -r f; do + if [ -f "$f" ]; then printf '%s\n' "$f"; fi + done +} + litellm_py_pattern='^litellm/.*\.py$' e2e_py_pattern='^tests/e2e/.*\.py$' spec_pattern='^(litellm/(proxy|types)/.*|ui/litellm-dashboard/(scripts/gen-api-types\.mjs|package\.json|package-lock\.json|src/lib/http/schema\.d\.ts))$' @@ -80,15 +86,17 @@ ui_eslint_pattern='^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs)$' litellm_py_files=$(scope_match "$litellm_py_pattern") e2e_py_files=$(scope_match "$e2e_py_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/' || true) +fmt_files=$(printf '%s\n' "$litellm_py_files" | grep -v '^litellm/enterprise/' | existing_files) # check-ui-api-types.yml triggers on any file under litellm/proxy or litellm/types # (Prisma schema and configs included, not just Python) plus the generator and its # lockfiles, so match that whole trigger set rather than a Python subset. spec_files=$(scope_match "$spec_pattern") # CI's frontend-lint runs prettier over a wider extension set than eslint; keep that # split so this flags exactly what the job would. -ui_prettier_files=$(scope_match "$ui_prettier_pattern") -ui_eslint_files=$(scope_match "$ui_eslint_pattern") +ui_prettier_changed=$(scope_match "$ui_prettier_pattern") +ui_eslint_changed=$(scope_match "$ui_eslint_pattern") +ui_prettier_files=$(printf '%s\n' "$ui_prettier_changed" | existing_files) +ui_eslint_files=$(printf '%s\n' "$ui_eslint_changed" | existing_files) # CI lints the committed tree, so with staged files this script predicts CI for # what you have STAGED (every trigger above reads `git diff --cached`). The tools @@ -116,7 +124,7 @@ if [ -n "$staged" ]; then } 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 "dashboard lint (prettier + eslint + lint budgets)" "$ui_prettier_pattern" "$ui_prettier_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" fi @@ -214,7 +222,7 @@ dashboard_checks() { lint_dashboard || { echo "✗ Dashboard lint failed. See above; format with: (cd ui/litellm-dashboard && npm run format)." >&2; return 1; } } -if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then +if [ -n "$ui_prettier_changed" ] || [ -n "$ui_eslint_changed" ]; then dash_log=$(mktemp) set -m dashboard_checks > "$dash_log" 2>&1 & diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py index 12b8d338b49..35d98903226 100644 --- a/tests/test_litellm/test_pre_commit_lint.py +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -181,6 +181,42 @@ def test_nothing_staged_includes_untracked_files_in_scope(tmp_path: Path) -> Non assert "linting Python" in proc.stdout +def test_nothing_staged_deletion_only_branch_triggers_checks(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + _set_base_ref(repo) + (repo / "litellm" / "foo.py").unlink() + _commit_all(repo, "delete module") + proc = _run(repo, bin_dir, {}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "nothing to check" not in proc.stdout + assert "litellm/foo.py" in proc.stdout + assert "linting Python" in proc.stdout + assert "ruff format --check" not in proc.stdout + + +def test_staged_deletion_triggers_checks_without_feeding_missing_files_to_tools(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + subprocess.run(["git", "rm", "-q", "litellm/foo.py"], cwd=repo, check=True) + proc = _run(repo, bin_dir, {}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "nothing staged" not in proc.stdout + assert "linting Python" in proc.stdout + assert "ruff format --check" not in proc.stdout + + +def test_deleted_dashboard_file_still_triggers_dashboard_lint(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + _set_base_ref(repo) + (repo / "ui" / "litellm-dashboard" / "src" / "app.ts").unlink() + _commit_all(repo, "delete dashboard file") + proc = _run(repo, bin_dir, {}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "linting dashboard" in proc.stdout + + def test_nothing_staged_and_no_changes_is_an_explicit_no_op(tmp_path: Path) -> None: repo, bin_dir = _sandbox(tmp_path) _commit_all(repo, "base")