From 75b447824214827c77c6554e3759907533cddbcc Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:00:03 -0700 Subject: [PATCH] fix(ci): run the risk gate from the base branch and tier moves, docs, and manifests correctly The workflow now runs on pull_request_target so the base branch's copy of the script and config scores every PR, forks included, and a PR can no longer edit the gate that scores it. The PR head is only ever read as a diff. Forks are passed --from-fork so the author factor is live The diff parser reads git renames, so a pure move counts zero lines and both paths take part in the paths and modules factors. The size factor counts only files outside the docs, tests, and model map tiers, and .only( joins the silenced-test markers Config: every pyproject.toml is always-human, the MCP auth, credential, OAuth, and discovery files are always-human, and the lint budget files are low. The last 400 staging merges now score 217 high, 139 medium, 44 low --- .github/risk-tiers.yml | 9 +- .github/scripts/risk_tier.py | 160 ++++++++++++-------- .github/workflows/risk-gate.yml | 53 ++++--- tests/test_litellm/test_github_risk_tier.py | 112 +++++++++++++- 4 files changed, 253 insertions(+), 81 deletions(-) diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml index 51110e058c6..ce7428e7b6b 100644 --- a/.github/risk-tiers.yml +++ b/.github/risk-tiers.yml @@ -22,9 +22,15 @@ paths: - "litellm/secret_managers/**" - "litellm/proxy/pass_through_endpoints/**" - "litellm/passthrough/**" + - "litellm/proxy/_experimental/mcp_server/auth/**" + - "litellm/proxy/_experimental/mcp_server/outbound_credentials/**" + - "litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py" + - "litellm/proxy/_experimental/mcp_server/*oauth*" + - "litellm/proxy/_experimental/mcp_server/*_flow.py" + - "litellm/proxy/_experimental/mcp_server/proxy_api_credentials.py" - ".github/**" - ".circleci/**" - - "pyproject.toml" + - "**/pyproject.toml" - "uv.lock" - "**/package*.json" - "**/Dockerfile*" @@ -35,6 +41,7 @@ paths: - "litellm/model_prices_and_context_window_backup.json" - "**/*.md" - "**/*.mdx" + - "*-budget.json" modules: medium_from: 2 diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py index 2996f780664..b210172ebed 100644 --- a/.github/scripts/risk_tier.py +++ b/.github/scripts/risk_tier.py @@ -9,20 +9,24 @@ from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path from types import MappingProxyType -from typing import Final, Literal +from typing import Final, Literal, TypeAlias import yaml from pydantic import BaseModel, ConfigDict, Field +from typing_extensions import ReadOnly, TypedDict -Tier = Literal["low", "medium", "high"] +Tier: TypeAlias = Literal["low", "medium", "high"] TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(") SKIP_RE: Final = re.compile( - r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b|\b(?:it|test|describe)\.skip\(\s*[\"'`]|\bx(?:it|test|describe)\(" + r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b" + r"|\b(?:it|test|describe)\.(?:skip\(\s*[\"'`]|only\()|\bx(?:it|test|describe)\(" ) ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(") GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)") DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git " +RENAME_FROM_RE: Final = re.compile(r"^rename from (.+)$") +RENAME_TO_RE: Final = re.compile(r"^rename to (.+)$") QUOTED_PATH_ESCAPE_RE: Final = re.compile(r'\\(?:([abfnrtv"\\])|([0-7]{3}))') QUOTED_PATH_ESCAPES: Final = MappingProxyType( {"a": "\a", "b": "\b", "f": "\f", "n": "\n", "r": "\r", "t": "\t", "v": "\v", '"': '"', "\\": "\\"} @@ -80,6 +84,22 @@ class PathMatcher: return any(pattern.fullmatch(path) for pattern in self.patterns) +@dataclass(frozen=True, slots=True) +class FileChange: + path: str + added_lines: tuple[str, ...] + deleted_lines: tuple[str, ...] + previous_path: str | None = None + + @property + def paths(self) -> tuple[str, ...]: + return (self.path,) if self.previous_path is None else (self.previous_path, self.path) + + @property + def line_count(self) -> int: + return len(self.added_lines) + len(self.deleted_lines) + + @dataclass(frozen=True, slots=True) class Rules: config: RiskConfig @@ -105,19 +125,11 @@ class Rules: return "low" return "medium" - def is_production(self, path: str) -> bool: - return self.path_tier(path) != "low" + def change_tier(self, change: FileChange) -> Tier: + return highest(tuple(self.path_tier(path) for path in change.paths)) - -@dataclass(frozen=True, slots=True) -class FileChange: - path: str - added_lines: tuple[str, ...] - deleted_lines: tuple[str, ...] - - @property - def line_count(self) -> int: - return len(self.added_lines) + len(self.deleted_lines) + def is_production(self, change: FileChange) -> bool: + return self.change_tier(change) != "low" @dataclass(frozen=True, slots=True) @@ -127,19 +139,31 @@ class Factor: reason: str +class FactorJson(TypedDict): + name: ReadOnly[str] + tier: ReadOnly[Tier] + reason: ReadOnly[str] + + +class VerdictJson(TypedDict): + tier: ReadOnly[Tier] + factors: ReadOnly[tuple[FactorJson, ...]] + summary: ReadOnly[str] + + @dataclass(frozen=True, slots=True) class Verdict: tier: Tier factors: tuple[Factor, ...] def summary_markdown(self) -> str: - rows = "\n".join(f"| {factor.name} | {factor.tier} | {factor.reason} |" for factor in self.factors) + rows: Final = "\n".join(f"| {factor.name} | {factor.tier} | {factor.reason} |" for factor in self.factors) return f"risk: {self.tier} (shadow mode, nothing is blocked)\n\n| factor | tier | why |\n| --- | --- | --- |\n{rows}\n" def to_json(self) -> str: - payload = { + payload: Final[VerdictJson] = { "tier": self.tier, - "factors": [{"name": f.name, "tier": f.tier, "reason": f.reason} for f in self.factors], + "factors": tuple(FactorJson(name=f.name, tier=f.tier, reason=f.reason) for f in self.factors), "summary": self.summary_markdown(), } return json.dumps(payload, indent=2) @@ -168,7 +192,7 @@ def load_config(path: Path) -> RiskConfig: def parse_diff(diff_text: str) -> tuple[FileChange, ...]: - blocks = ("\n" + diff_text).split(DIFF_BLOCK_SEPARATOR)[1:] + blocks: Final = ("\n" + diff_text).split(DIFF_BLOCK_SEPARATOR)[1:] return tuple(_parse_block(block) for block in blocks) @@ -179,30 +203,39 @@ def _unquote_git_path(quoted: str) -> str: return QUOTED_PATH_ESCAPE_RE.sub(decode, quoted[1:-1]) +def _unquote(path: str) -> str: + return _unquote_git_path(path) if path.startswith('"') else path + + def _header_path(header: str) -> str: - one_side = header[: (len(header) - 1) // 2] - unquoted = _unquote_git_path(one_side) if one_side.startswith('"') else one_side - return unquoted.removeprefix("a/") + one_side: Final = header[: (len(header) - 1) // 2] + return _unquote(one_side).removeprefix("a/") + + +def _rename_side(extended_header: Sequence[str], pattern: re.Pattern[str]) -> str | None: + return next((_unquote(match[1]) for line in extended_header if (match := pattern.match(line))), None) def _parse_block(block: str) -> FileChange: header, _, body = block.partition("\n") - path = _header_path(header) - lines = body.split("\n") - first_hunk = next((index for index, line in enumerate(lines) if line.startswith("@@")), len(lines)) - hunk_lines = lines[first_hunk:] + lines: Final = body.split("\n") + first_hunk: Final = next((index for index, line in enumerate(lines) if line.startswith("@@")), len(lines)) + extended_header: Final = lines[:first_hunk] + hunk_lines: Final = lines[first_hunk:] + renamed_to: Final = _rename_side(extended_header, RENAME_TO_RE) return FileChange( - path=path, + path=renamed_to if renamed_to is not None else _header_path(header), added_lines=tuple(line[1:] for line in hunk_lines if line.startswith("+")), deleted_lines=tuple(line[1:] for line in hunk_lines if line.startswith("-")), + previous_path=_rename_side(extended_header, RENAME_FROM_RE) if renamed_to is not None else None, ) def module_key(path: str) -> str: - parts = path.split("/") + parts: Final = path.split("/") if parts[0] != "litellm": return parts[0] - depth = 3 if len(parts) > 3 else 2 + depth: Final = 3 if len(parts) > 3 else 2 return "/".join(parts[:depth]) @@ -215,17 +248,20 @@ def highest(tiers: Sequence[Tier]) -> Tier: def _listed(paths: Sequence[str]) -> str: - shown = ", ".join(f"`{path}`" for path in paths[:MAX_LISTED_PATHS]) - rest = len(paths) - MAX_LISTED_PATHS + shown: Final = ", ".join(f"`{path}`" for path in paths[:MAX_LISTED_PATHS]) + rest: Final = len(paths) - MAX_LISTED_PATHS return f"{shown} and {rest} more" if rest > 0 else shown def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - tier = highest([rules.path_tier(change.path) for change in changes]) - matching = [change.path for change in changes if rules.path_tier(change.path) == tier] + tier: Final = highest(tuple(rules.change_tier(change) for change in changes)) + matching: Final = tuple(change for change in changes if rules.change_tier(change) == tier) match tier: case "high": - return Factor("paths", "high", f"always-human: {_listed(matching)}") + always_human: Final = tuple( + path for change in matching for path in change.paths if rules.path_tier(path) == "high" + ) + return Factor("paths", "high", f"always-human: {_listed(always_human)}") case "medium": return Factor("paths", "medium", f"{len(matching)} file(s) outside the docs, tests, and model map tiers") case "low": @@ -233,9 +269,13 @@ def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: def modules_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - modules = sorted({module_key(change.path) for change in changes if rules.is_production(change.path)}) - count = len(modules) - tier: Tier = ( + modules: Final = tuple( + sorted( + frozenset(module_key(path) for change in changes for path in change.paths if rules.path_tier(path) != "low") + ) + ) + count: Final = len(modules) + tier: Final[Tier] = ( "high" if count >= rules.config.modules.high_from else "medium" @@ -246,32 +286,34 @@ def modules_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: def size_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - counted = [change for change in changes if not rules.size_ignored.matches(change.path)] - lines = sum(change.line_count for change in counted) - files = len(counted) - limits = rules.config.size - tier: Tier = ( + counted: Final = tuple( + change for change in changes if rules.is_production(change) and not rules.size_ignored.matches(change.path) + ) + lines: Final = sum(change.line_count for change in counted) + files: Final = len(counted) + limits: Final = rules.config.size + tier: Final[Tier] = ( "low" if lines < limits.low.lines_under and files <= limits.low.files_up_to else "medium" if lines < limits.medium.lines_under and files <= limits.medium.files_up_to else "high" ) - return Factor("size", tier, f"{lines} line(s) across {files} file(s)") + return Factor("size", tier, f"{lines} line(s) across {files} file(s) outside the docs, tests, and model map tiers") def _net(changes: Sequence[FileChange], pattern: re.Pattern[str]) -> int: - added = sum(1 for change in changes for line in change.added_lines if pattern.search(line)) - deleted = sum(1 for change in changes for line in change.deleted_lines if pattern.search(line)) + added: Final = sum(1 for change in changes for line in change.added_lines if pattern.search(line)) + deleted: Final = sum(1 for change in changes for line in change.deleted_lines if pattern.search(line)) return added - deleted def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - test_changes = [change for change in changes if rules.test_files.matches(change.path)] - net_tests = _net(test_changes, TEST_DEF_RE) - net_skips = _net(test_changes, SKIP_RE) - net_asserts = _net(test_changes, ASSERT_RE) - production_changed = any(rules.is_production(change.path) for change in changes) + test_changes: Final = tuple(change for change in changes if rules.test_files.matches(change.path)) + net_tests: Final = _net(test_changes, TEST_DEF_RE) + net_skips: Final = _net(test_changes, SKIP_RE) + net_asserts: Final = _net(test_changes, ASSERT_RE) + production_changed: Final = any(rules.is_production(change) for change in changes) if net_tests < 0: return Factor("tests", "high", f"{-net_tests} test(s) removed") if net_skips > 0: @@ -294,19 +336,19 @@ def author_factor(author: str, from_fork: bool) -> Factor: def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict: - factors = ( + factors: Final = ( paths_factor(changes, rules), modules_factor(changes, rules), size_factor(changes, rules), tests_factor(changes, rules), author_factor(author, from_fork), ) - return Verdict(highest([factor.tier for factor in factors]), factors) + return Verdict(highest(tuple(factor.tier for factor in factors)), factors) def git_diff(repo: Path, base: str, head: str) -> str: - completed = subprocess.run( - ["git", "-c", "core.quotePath=false", "diff", "--no-renames", "--no-ext-diff", "-U0", base, head], + completed: Final = subprocess.run( + ("git", "-c", "core.quotePath=false", "diff", "--find-renames", "--no-ext-diff", "-U0", base, head), cwd=repo, capture_output=True, encoding="utf-8", @@ -317,7 +359,7 @@ def git_diff(repo: Path, base: str, head: str) -> str: def build_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Compute a pull request's floor risk tier from its diff") + parser: Final = argparse.ArgumentParser(description="Compute a pull request's floor risk tier from its diff") parser.add_argument("--config", type=Path, required=True) parser.add_argument("--base", required=True) parser.add_argument("--head", required=True) @@ -340,10 +382,10 @@ class CliArgs(BaseModel): def main(argv: Sequence[str] | None = None) -> int: - args = CliArgs.model_validate(vars(build_parser().parse_args(argv))) - rules = Rules.from_config(load_config(args.config)) - changes = parse_diff(git_diff(args.repo, args.base, args.head)) - verdict = classify(changes, args.author, args.from_fork, rules) + args: Final = CliArgs.model_validate(vars(build_parser().parse_args(argv))) + rules: Final = Rules.from_config(load_config(args.config)) + changes: Final = parse_diff(git_diff(args.repo, args.base, args.head)) + verdict: Final = classify(changes, args.author, args.from_fork, rules) if args.json_out is not None: args.json_out.write_text(verdict.to_json(), encoding="utf-8") sys.stdout.write(verdict.summary_markdown()) diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index ffa246ddbca..0e3645b83a7 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -1,12 +1,13 @@ name: risk-gate -on: - pull_request: +on: # zizmor: ignore[dangerous-triggers] runs the base branch's copy of the gate only; the PR's diff is read as data and never executed + pull_request_target: types: [opened, synchronize, reopened, ready_for_review] branches: - litellm_internal_staging -permissions: {} +permissions: + contents: read concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number }} @@ -15,9 +16,8 @@ concurrency: jobs: floor: name: risk-gate floor (shadow) - if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 permissions: contents: read pull-requests: write @@ -25,30 +25,43 @@ jobs: steps: - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 with: - fetch-depth: 2 persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + - name: Fetch the pull request head and its merge base + id: revisions + env: + GH_TOKEN: ${{ github.token }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + merge_base="$(gh api "repos/${GITHUB_REPOSITORY}/compare/${BASE_SHA}...${HEAD_SHA}" --jq '.merge_base_commit.sha')" + git fetch --no-tags --depth=1 origin "$merge_base" "$HEAD_SHA" + echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT" + + - name: Set up uv + uses: ./.github/actions/setup-uv-with-retries with: - python-version: "3.12" + version: "0.10.9" - name: Compute the floor tier with the base branch's copy of the gate - shell: bash env: + MERGE_BASE: ${{ steps.revisions.outputs.merge_base }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} + FROM_FORK: ${{ github.event.pull_request.head.repo.full_name != github.repository }} run: | - python -m pip install "pyyaml==6.0.3" "pydantic==2.13.4" - for gate_file in scripts/risk_tier.py risk-tiers.yml; do - target="${RUNNER_TEMP}/$(basename "${gate_file}")" - git show "HEAD^1:.github/${gate_file}" > "${target}" 2>/dev/null || cp ".github/${gate_file}" "${target}" - done - python "${RUNNER_TEMP}/risk_tier.py" \ - --config "${RUNNER_TEMP}/risk-tiers.yml" \ - --base HEAD^1 \ - --head HEAD \ - --author "${PR_AUTHOR}" \ + fork_flag=() + if [ "$FROM_FORK" = "true" ]; then + fork_flag=(--from-fork) + fi + uv run --frozen python .github/scripts/risk_tier.py \ + --config .github/risk-tiers.yml \ + --base "$MERGE_BASE" \ + --head "$HEAD_SHA" \ + --author "$PR_AUTHOR" \ + "${fork_flag[@]}" \ --json-out "${RUNNER_TEMP}/risk.json" \ - | tee -a "${GITHUB_STEP_SUMMARY}" + | tee -a "$GITHUB_STEP_SUMMARY" - name: Publish the risk-gate check run and the risk label uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index 722deb0603b..eb905a2e25a 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -2,6 +2,7 @@ from __future__ import annotations import importlib.util import json +import os import subprocess import sys from collections.abc import Sequence @@ -47,6 +48,15 @@ def _lines(count: int, prefix: str = "x = ") -> tuple[str, ...]: return tuple(f"{prefix}{index}" for index in range(count)) +def _rename_diff(old: str, new: str, deleted: Sequence[str] = (), added: Sequence[str] = ()) -> str: + header = f"diff --git a/{old} b/{new}\nsimilarity index 90%\nrename from {old}\nrename to {new}\n" + if not deleted and not added: + return header.replace("90%", "100%") + hunk = f"index 0000000..1111111 100644\n--- a/{old}\n+++ b/{new}\n@@ -1,{len(deleted)} +1,{len(added)} @@\n" + body = "".join(f"-{line}\n" for line in deleted) + "".join(f"+{line}\n" for line in added) + return header + hunk + body + + NEW_TEST = ("def test_regression():", " assert True") @@ -149,7 +159,49 @@ def test_generated_files_do_not_count_toward_size(risk_tier, rules): ) factor = _factor(_verdict(risk_tier, rules, diff), "size") assert factor.tier == "low" - assert factor.reason == "1 line(s) across 1 file(s)" + assert factor.reason == "1 line(s) across 1 file(s) outside the docs, tests, and model map tiers" + + +def test_docs_and_tests_do_not_count_toward_size(risk_tier, rules): + diff = ( + _file_diff("docs/my-website/docs/proxy/guide.md", added=_lines(5000, prefix="line ")) + + _file_diff("tests/test_litellm/test_x.py", added=_lines(500) + NEW_TEST) + + _file_diff("litellm/llms/anthropic/chat/x.py", added=("x = 1",)) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "size") + assert factor.tier == "low" + assert factor.reason == "1 line(s) across 1 file(s) outside the docs, tests, and model map tiers" + + +def test_pure_move_counts_no_lines_toward_size(risk_tier, rules): + diff = _rename_diff("litellm/llms/anthropic/chat/old.py", "litellm/llms/anthropic/chat/new.py") + factor = _factor(_verdict(risk_tier, rules, diff), "size") + assert factor.tier == "low" + assert factor.reason == "0 line(s) across 1 file(s) outside the docs, tests, and model map tiers" + + +def test_move_with_edits_counts_only_the_edited_lines(risk_tier, rules): + diff = _rename_diff( + "litellm/llms/anthropic/chat/old.py", + "litellm/llms/anthropic/chat/new.py", + deleted=_lines(2), + added=_lines(2, prefix="y = "), + ) + assert _factor(_verdict(risk_tier, rules, diff), "size").reason.startswith("4 line(s) across 1 file(s)") + + +def test_move_out_of_an_always_human_path_stays_high(risk_tier, rules): + diff = _rename_diff("litellm/proxy/auth/old.py", "litellm/proxy/common_utils/old.py") + factor = _factor(_verdict(risk_tier, rules, diff), "paths") + assert factor.tier == "high" + assert factor.reason == "always-human: `litellm/proxy/auth/old.py`" + + +def test_move_into_an_always_human_path_is_high(risk_tier, rules): + diff = _rename_diff("litellm/proxy/utils/new.py", "litellm/proxy/auth/new.py") + factor = _factor(_verdict(risk_tier, rules, diff), "paths") + assert factor.tier == "high" + assert factor.reason == "always-human: `litellm/proxy/auth/new.py`" def test_removed_test_function_is_high(risk_tier, rules): @@ -231,6 +283,8 @@ def test_production_change_without_any_test_is_medium(risk_tier, rules): (('it("hides the notice", () => {', " expect(screen.queryByText(notice)).toBeNull();", "});"), "low"), (('it.skip("hides the notice", () => {', "});"), "high"), (("test.skip('hides the notice', async () => {", "});"), "high"), + (('it.only("hides the notice", () => {', "});"), "high"), + (('describe.only("login", () => {', "});"), "high"), (('xit("hides the notice", () => {', "});"), "high"), ( ( @@ -279,6 +333,16 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("ui/litellm-dashboard/package.json", "high"), ("scripts/type_check_gate.py", "high"), (".github/risk-tiers.yml", "high"), + ("enterprise/pyproject.toml", "high"), + ("litellm-proxy-extras/pyproject.toml", "high"), + ("litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py", "high"), + ("litellm/proxy/_experimental/mcp_server/outbound_credentials/x.py", "high"), + ("litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py", "high"), + ("litellm/proxy/_experimental/mcp_server/oauth2_token_cache.py", "high"), + ("litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py", "high"), + ("litellm/proxy/_experimental/mcp_server/bridge_token_flow.py", "high"), + ("litellm/proxy/_experimental/mcp_server/proxy_api_credentials.py", "high"), + ("litellm/proxy/_experimental/mcp_server/server.py", "medium"), ("enterprise/litellm_enterprise/proxy/hooks/x.py", "high"), ("enterprise/litellm_enterprise/proxy/auth/x.py", "high"), ("enterprise/litellm_enterprise/proxy/management_endpoints/x.py", "high"), @@ -297,6 +361,8 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("model_prices_and_context_window.json", "low"), ("litellm/model_prices_and_context_window_backup.json", "low"), ("litellm/proxy/README.md", "low"), + ("type-discipline-budget.json", "low"), + ("ruff-strict-budget.json", "low"), ], ) def test_path_tier_from_the_checked_in_config(rules, path, expected): @@ -335,6 +401,27 @@ def test_parse_diff_of_an_empty_diff_is_empty(risk_tier): assert risk_tier.parse_diff("") == () +def test_parse_diff_reads_a_rename_as_one_change_with_both_paths(risk_tier): + diff = _rename_diff("litellm/a/very_long_old_name.py", "litellm/b/new.py", deleted=("x = 1",), added=("x = 2",)) + changes = risk_tier.parse_diff(diff) + assert len(changes) == 1 + assert changes[0].path == "litellm/b/new.py" + assert changes[0].previous_path == "litellm/a/very_long_old_name.py" + assert changes[0].paths == ("litellm/a/very_long_old_name.py", "litellm/b/new.py") + assert changes[0].line_count == 2 + + +def test_parse_diff_unquotes_renamed_paths(risk_tier): + diff = ( + 'diff --git "a/docs/we\\"ird.md" b/docs/plain.md\n' + "similarity index 100%\n" + 'rename from "docs/we\\"ird.md"\n' + "rename to docs/plain.md\n" + ) + changes = risk_tier.parse_diff(diff) + assert changes[0].paths == ('docs/we"ird.md', "docs/plain.md") + + def test_parse_diff_decodes_git_quoted_paths_instead_of_dropping_them(risk_tier): diff = ( "diff --git a/docs/plain.md b/docs/plain.md\n" @@ -370,6 +457,7 @@ def _git(repo: Path, *args: str) -> str: capture_output=True, text=True, env={ + "PATH": os.environ["PATH"], "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@x", "GIT_COMMITTER_NAME": "t", @@ -443,3 +531,25 @@ def test_git_quoted_filename_still_reaches_the_paths_factor(risk_tier, rules, tm assert changes[0].added_lines == ("def f():", " return 1") verdict = risk_tier.classify(changes, DEVIN, False, rules) assert _factor(verdict, "paths").tier == "high" + + +def test_git_pure_move_is_one_zero_line_change(risk_tier, rules, tmp_path): + repo = tmp_path / "repo" + (repo / "litellm" / "proxy" / "auth").mkdir(parents=True) + _git(tmp_path, "init", "-q", "-b", "main", "repo") + (repo / "litellm" / "proxy" / "auth" / "checks.py").write_text("".join(f"x{i} = {i}\n" for i in range(300))) + _git(repo, "add", ".") + _git(repo, "commit", "-q", "-m", "base") + base = _git(repo, "rev-parse", "HEAD") + (repo / "litellm" / "proxy" / "utils").mkdir() + _git(repo, "mv", "litellm/proxy/auth/checks.py", "litellm/proxy/utils/checks.py") + _git(repo, "commit", "-q", "-m", "move") + head = _git(repo, "rev-parse", "HEAD") + + changes = risk_tier.parse_diff(risk_tier.git_diff(repo, base, head)) + + assert [change.paths for change in changes] == [("litellm/proxy/auth/checks.py", "litellm/proxy/utils/checks.py")] + assert changes[0].line_count == 0 + verdict = risk_tier.classify(changes, DEVIN, False, rules) + assert _factor(verdict, "paths").tier == "high" + assert _factor(verdict, "size").tier == "low"