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"