mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
Merge pull request #35869 from BerriAI/litellm_gate_owns_basedpyright_heap
fix(lint): move the basedpyright heap flag into the type check gate
This commit is contained in:
commit
faf3c51469
5 changed files with 95 additions and 26 deletions
3
.github/workflows/test-linting.yml
vendored
3
.github/workflows/test-linting.yml
vendored
|
|
@ -104,9 +104,8 @@ jobs:
|
|||
- name: Check basedpyright budget (delta vs base)
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
NODE_OPTIONS: --max-old-space-size=12288
|
||||
run: |
|
||||
(uv run --no-sync basedpyright --outputjson || true) | uv run --no-sync python scripts/type_check_gate.py --base "$BASE_SHA"
|
||||
uv run --no-sync python scripts/type_check_gate.py --base "$BASE_SHA"
|
||||
|
||||
- name: Check tests/e2e basedpyright (zero errors)
|
||||
env:
|
||||
|
|
|
|||
6
Makefile
6
Makefile
|
|
@ -176,10 +176,8 @@ lint-ruff-FULL-dev: install-dev
|
|||
if [ -n "$$files" ]; then echo "$$files" | xargs $(UV_RUN) ruff check; \
|
||||
else echo "No changed .py files to check."; fi
|
||||
|
||||
lint-basedpyright lint-basedpyright-budget-update: export NODE_OPTIONS := --max-old-space-size=12288
|
||||
|
||||
lint-basedpyright: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
||||
($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --base origin/litellm_internal_staging
|
||||
$(UV_RUN) python scripts/type_check_gate.py --base origin/litellm_internal_staging
|
||||
|
||||
lint-e2e-basedpyright: $(LINT_E2E_DEP_INSTALL)
|
||||
$(UV_RUN) basedpyright tests/e2e
|
||||
|
|
@ -192,7 +190,7 @@ lint-type-discipline: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
|||
# --update lowers each limit by what this branch fixed since its branch point, so
|
||||
# it needs the base ref fetched to resolve the merge-base.
|
||||
lint-basedpyright-budget-update: install-dev lint-fetch-base
|
||||
($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --update
|
||||
$(UV_RUN) python scripts/type_check_gate.py --update
|
||||
|
||||
lint-format: format-check
|
||||
|
||||
|
|
|
|||
|
|
@ -12,12 +12,16 @@ a red once two PRs each land near the limit and their sum crosses it: the
|
|||
bystander's count equals its base, so it is spared, while any PR that actually
|
||||
grows the rule past its limit still fails.
|
||||
|
||||
Head counts are read from stdin (the caller runs basedpyright once and pipes
|
||||
``--outputjson`` in). The base count only matters once some rule is over its
|
||||
limit, so when none is the base pass is skipped outright. When it is needed, it
|
||||
is a second basedpyright pass over a detached worktree at the merge-base, run
|
||||
under the same environment so import resolution matches, and its per-rule
|
||||
counts are cached under the repo's git common dir keyed by merge-base commit,
|
||||
The gate runs basedpyright itself, for both the head and the base pass, with
|
||||
``NODE_OPTIONS`` raised to the heap this repo needs: basedpyright's node
|
||||
process OOMs at the ~4 GB default, and when callers had to remember the flag,
|
||||
every hand-copied pipeline (Makefile, CI, a dev running the recipe by hand)
|
||||
was one forgotten env line away from an 80-second crash. The base count only
|
||||
matters once some rule is over its limit, so when none is the base pass is
|
||||
skipped outright. When it is needed, it is a second basedpyright pass over a
|
||||
detached worktree at the merge-base, run under the same environment so import
|
||||
resolution matches, and its per-rule counts are cached under the repo's git
|
||||
common dir keyed by merge-base commit,
|
||||
``pyrightconfig.json``, and ``uv.lock``, so re-runs against the same branch
|
||||
point pay for it once. ``--update`` ratchets each rule's ``limit`` down by the
|
||||
number of errors this branch fixed relative to its branch point (the merge-base),
|
||||
|
|
@ -51,6 +55,11 @@ UV_LOCK = REPO_ROOT / "uv.lock"
|
|||
DEFAULT_BASE = "origin/litellm_internal_staging"
|
||||
CACHE_FILE_PREFIX = "basedpyright-base-"
|
||||
|
||||
# basedpyright's node process needs more than the ~4 GB default heap on this
|
||||
# repo; appended last so it wins node's last-flag-wins resolution over any
|
||||
# caller-set value while preserving the caller's other NODE_OPTIONS flags.
|
||||
NODE_HEAP_OPTION = "--max-old-space-size=12288"
|
||||
|
||||
# Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated.
|
||||
UNCODED = "<uncoded>"
|
||||
|
||||
|
|
@ -107,6 +116,29 @@ def _run(cmd: list[str], cwd: Path = REPO_ROOT) -> str:
|
|||
return proc.stdout
|
||||
|
||||
|
||||
def node_options_with_heap(base_env: Mapping[str, str]) -> str:
|
||||
return f"{base_env.get('NODE_OPTIONS', '')} {NODE_HEAP_OPTION}".strip()
|
||||
|
||||
|
||||
def run_basedpyright(cwd: Path = REPO_ROOT) -> str:
|
||||
"""One basedpyright pass over `cwd` with the raised node heap exported.
|
||||
|
||||
Exit 0 (clean) and 1 (errors found) are both output-bearing runs; anything
|
||||
else is a crash and fails loudly instead of reading as zero errors."""
|
||||
exe = shutil.which("basedpyright") or "basedpyright"
|
||||
proc = subprocess.run(
|
||||
[exe, "--outputjson"],
|
||||
cwd=cwd,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env={**os.environ, "NODE_OPTIONS": node_options_with_heap(os.environ)},
|
||||
)
|
||||
if proc.returncode not in (0, 1):
|
||||
sys.stderr.write(proc.stderr)
|
||||
raise SystemExit(f"basedpyright exited {proc.returncode}")
|
||||
return proc.stdout
|
||||
|
||||
|
||||
def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str:
|
||||
"""The snapshot commit base counts are measured at: merge-base(base_ref, HEAD),
|
||||
made aware of an in-progress merge. Mid-merge, HEAD is still the pre-merge tip,
|
||||
|
|
@ -147,13 +179,9 @@ def base_counts(ref: str) -> dict[str, int]:
|
|||
"""basedpyright error counts per rule for the merge-base tree. The head
|
||||
config is copied in so the base is judged by today's rules, and the run uses
|
||||
the head environment's basedpyright (on PATH) so imports resolve the same."""
|
||||
exe = shutil.which("basedpyright") or "basedpyright"
|
||||
with _temp_worktree(ref) as worktree:
|
||||
shutil.copy(PYRIGHT_CONFIG, worktree / "pyrightconfig.json")
|
||||
proc = subprocess.run(
|
||||
[exe, "--outputjson"], cwd=worktree, capture_output=True, text=True
|
||||
)
|
||||
return count_basedpyright(proc.stdout, root=worktree)
|
||||
return count_basedpyright(run_basedpyright(worktree), root=worktree)
|
||||
|
||||
|
||||
def over_ceiling(
|
||||
|
|
@ -278,9 +306,10 @@ def is_vacuous_run(
|
|||
counts: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
|
||||
) -> bool:
|
||||
"""True when nothing was parsed but the budget expects errors -- the
|
||||
signature of a type checker that crashed or produced no output. The CI pipe
|
||||
swallows the tool's exit code (`tool || true`), so without this guard an
|
||||
empty run would clear every limit and pass silently."""
|
||||
signature of a type checker that produced no output. `run_basedpyright`
|
||||
already fails crash exit codes, so this guards the remaining case: a run
|
||||
that exits cleanly while emitting nothing, which would otherwise clear
|
||||
every limit and pass silently."""
|
||||
return not counts and any(spec["limit"] for spec in budget.values())
|
||||
|
||||
|
||||
|
|
@ -308,7 +337,7 @@ def ratcheted_budget(
|
|||
def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None:
|
||||
"""Ratchet each rule's limit down by the errors this branch fixed.
|
||||
|
||||
`current` is the working-tree count (piped in); the reference count comes
|
||||
`current` is the working-tree count; the reference count comes
|
||||
from a second basedpyright pass over a detached worktree at the branch point
|
||||
(the merge-base with `base_ref`), so a branch's fixes tighten its own ceilings
|
||||
by exactly what they cleared since it diverged, and limits never rise.
|
||||
|
|
@ -324,9 +353,8 @@ def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None
|
|||
)
|
||||
|
||||
|
||||
def cmd_check(base_ref: str) -> None:
|
||||
def cmd_check(head: Mapping[str, int], base_ref: str) -> None:
|
||||
budget = json.loads(BUDGET_PATH.read_text())
|
||||
head = count_basedpyright(sys.stdin.read())
|
||||
if is_vacuous_run(head, budget):
|
||||
expected = sum(spec["limit"] for spec in budget.values())
|
||||
print(
|
||||
|
|
@ -374,10 +402,11 @@ def main() -> None:
|
|||
parser.add_argument("--base", default=DEFAULT_BASE)
|
||||
parser.add_argument("--update", action="store_true")
|
||||
args = parser.parse_args()
|
||||
head = count_basedpyright(run_basedpyright())
|
||||
if args.update:
|
||||
cmd_update(count_basedpyright(sys.stdin.read()), args.base)
|
||||
cmd_update(head, args.base)
|
||||
else:
|
||||
cmd_check(args.base)
|
||||
cmd_check(head, args.base)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -70,6 +71,48 @@ def test_symlinked_root_keeps_diagnostics_in_tree(tmp_path):
|
|||
assert gate.count_basedpyright(payload, root=link) == {"reportArgumentType": 1}
|
||||
|
||||
|
||||
def test_node_options_with_heap_sets_the_flag_in_a_bare_env():
|
||||
assert gate.node_options_with_heap({}) == gate.NODE_HEAP_OPTION
|
||||
|
||||
|
||||
def test_node_options_with_heap_appends_after_caller_flags_so_it_wins():
|
||||
# node resolves a repeated --max-old-space-size last-wins, so ours must come
|
||||
# after any caller-set value while keeping their other flags.
|
||||
merged = gate.node_options_with_heap(
|
||||
{"NODE_OPTIONS": "--max-old-space-size=4096 --no-warnings"}
|
||||
)
|
||||
assert merged == f"--max-old-space-size=4096 --no-warnings {gate.NODE_HEAP_OPTION}"
|
||||
|
||||
|
||||
def _stub_basedpyright(tmp_path, monkeypatch, script_body):
|
||||
stub = tmp_path / "basedpyright"
|
||||
stub.write_text(f"#!/bin/sh\n{script_body}\n")
|
||||
stub.chmod(0o755)
|
||||
monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
|
||||
|
||||
|
||||
def test_run_basedpyright_exports_the_raised_heap_to_the_child(tmp_path, monkeypatch):
|
||||
captured = tmp_path / "node_options.txt"
|
||||
_stub_basedpyright(
|
||||
tmp_path,
|
||||
monkeypatch,
|
||||
f'echo "$NODE_OPTIONS" > "{captured}"\necho \'{{"generalDiagnostics": []}}\'',
|
||||
)
|
||||
monkeypatch.delenv("NODE_OPTIONS", raising=False)
|
||||
assert json.loads(gate.run_basedpyright(cwd=tmp_path)) == {"generalDiagnostics": []}
|
||||
assert captured.read_text().strip() == gate.NODE_HEAP_OPTION
|
||||
|
||||
|
||||
def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path, monkeypatch):
|
||||
import pytest
|
||||
|
||||
# 134 is SIGABRT, what node dies with on a heap OOM; it must never read as a
|
||||
# clean zero-error run.
|
||||
_stub_basedpyright(tmp_path, monkeypatch, "exit 134")
|
||||
with pytest.raises(SystemExit):
|
||||
gate.run_basedpyright(cwd=tmp_path)
|
||||
|
||||
|
||||
def test_at_or_under_ceiling_passes():
|
||||
budget = {"no-any-return": {"limit": 5}}
|
||||
assert gate.evaluate({"no-any-return": 5}, {}, budget) == []
|
||||
|
|
|
|||
2
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
2
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -24004,7 +24004,7 @@ export interface components {
|
|||
* @description Default role assigned to new users created
|
||||
* @default internal_user_viewer
|
||||
*/
|
||||
user_role: ("internal_user" | "internal_user_viewer" | "proxy_admin" | "proxy_admin_viewer") | null;
|
||||
user_role: ("proxy_admin" | "proxy_admin_viewer" | "internal_user" | "internal_user_viewer") | null;
|
||||
};
|
||||
/**
|
||||
* DefaultTeamSSOParams
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue