From a550b95d70a7c6e538aac9cdc7ebfb949c38051e Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:40:48 -0700 Subject: [PATCH] ci: skip cost map file checks on PRs that leave the cost map untouched (#42406) Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com> --- ci_cd/cost_map_guard.py | 41 +++++++++--- tests/test_litellm/test_cost_map_guard.py | 78 +++++++++++++++++++++++ 2 files changed, 110 insertions(+), 9 deletions(-) diff --git a/ci_cd/cost_map_guard.py b/ci_cd/cost_map_guard.py index 50aa40ba220..5842cf6f1ac 100644 --- a/ci_cd/cost_map_guard.py +++ b/ci_cd/cost_map_guard.py @@ -1,8 +1,11 @@ """Guard the cost map on pull requests. -Every pull request gets the file checks: the three cost map files parse, the backup copy matches the root file, -and the JSON schema is in sync and validates the map. Pull requests from the cost map sync bot (branches named -litellm_cost_map_sync_*) additionally may only touch those three files and may only add or update models. +Every pull request whose diff against its merge base touches one of the three cost map files gets the file +checks: the files parse, the backup copy matches the root file, and the JSON schema is in sync and validates the +map. A pull request that leaves all three untouched skips them, since merging it keeps the base branch's copies +and its head tree only carries whatever state the branch was cut from. Pull requests from the cost map sync bot +(branches named litellm_cost_map_sync_*) always get the file checks and additionally may only touch those three +files and may only add or update models. """ from __future__ import annotations @@ -108,20 +111,37 @@ def _bot_failures(base: Snapshot, head_map: CostMap, changed_files: Sequence[str ) +def touches_cost_map(changed_files: Sequence[str]) -> bool: + return any(path in GUARDED_PATHS for path in changed_files) + + +def contract_for(bot: bool, changed_files: Sequence[str]) -> str: + if bot: + return "bot contract enforced" + return "human PR, file checks only" if touches_cost_map(changed_files) else "human PR, cost map untouched" + + def guard_failures(base: Snapshot, head: Snapshot, changed_files: Sequence[str], bot: bool) -> tuple[str, ...]: + if not bot and not touches_cost_map(changed_files): + return () head_map: Final = _parse_object(head.cost_map, COST_MAP_PATH) if isinstance(head_map, str): return (head_map,) return (*_file_failures(head, head_map), *(_bot_failures(base, head_map, changed_files) if bot else ())) -def _git(*args: str) -> str: +def _git(*args: str) -> str | None: result: Final = subprocess.run(("git", *args), check=False, capture_output=True, text=True) - return result.stdout if result.returncode == 0 else "" + return result.stdout if result.returncode == 0 else None def snapshot(revision: str) -> Snapshot: - return Snapshot(*(_git("show", f"{revision}:{path}") for path in GUARDED_PATHS)) + return Snapshot(*(_git("show", f"{revision}:{path}") or "" for path in GUARDED_PATHS)) + + +def changed_files(base: str, head: str) -> tuple[str, ...] | None: + diff: Final = _git("diff", "--name-only", "--no-renames", base, head) + return None if diff is None else tuple(diff.splitlines()) def main(argv: Sequence[str]) -> int: @@ -131,9 +151,12 @@ def main(argv: Sequence[str]) -> int: parser.add_argument("--head-ref", required=True, help="head branch name of the pull request") args: Final = parser.parse_args(argv) bot: Final = args.head_ref.startswith(BOT_BRANCH_PREFIX) - changed_files: Final = tuple(_git("diff", "--name-only", args.base, args.head).splitlines()) - failures: Final = guard_failures(snapshot(args.base), snapshot(args.head), changed_files, bot) - contract: Final = "bot contract enforced" if bot else "human PR, file checks only" + changed: Final = changed_files(args.base, args.head) + if changed is None: + print(f"cost map guard failed: git diff {args.base} {args.head} failed, so the changed files are unknown") + return 1 + failures: Final = guard_failures(snapshot(args.base), snapshot(args.head), changed, bot) + contract: Final = contract_for(bot, changed) if failures: print(f"cost map guard failed ({contract}):") print("\n".join(f"- {failure}" for failure in failures)) diff --git a/tests/test_litellm/test_cost_map_guard.py b/tests/test_litellm/test_cost_map_guard.py index 1b4330ed62c..1595bd9864f 100644 --- a/tests/test_litellm/test_cost_map_guard.py +++ b/tests/test_litellm/test_cost_map_guard.py @@ -114,6 +114,40 @@ def test_unclassified_entry_key_is_reported() -> None: assert "Unclassified keys" in failure and "weird_thing" in failure +STALE_HEAD: Final = _snapshot(BASE_MAP, backup=_serialize({**BASE_MAP, "openrouter/b": _entry(3e-06)}), schema="{}") +CODE_ONLY: Final = ( + "litellm/utils.py", + "tests/test_litellm/test_utils.py", + "docs/model_prices_and_context_window.json", +) + + +def test_human_pr_that_leaves_the_cost_map_alone_skips_the_file_checks() -> None: + unparseable: Final = guard.Snapshot(cost_map="{not json", backup="", schema="") + assert _failures(STALE_HEAD, changed_files=CODE_ONLY, bot=False) == () + assert _failures(STALE_HEAD, changed_files=(), bot=False) == () + assert _failures(unparseable, changed_files=CODE_ONLY, bot=False) == () + + +@pytest.mark.parametrize("guarded_path", guard.GUARDED_PATHS) +def test_touching_any_cost_map_file_keeps_the_file_checks(guarded_path: str) -> None: + failures: Final = _failures(STALE_HEAD, changed_files=(*CODE_ONLY, guarded_path), bot=False) + assert [failure for failure in failures if failure.startswith(guard.BACKUP_PATH)] + assert [failure for failure in failures if failure.startswith(guard.SCHEMA_PATH)] + + +def test_bot_pr_always_gets_the_file_checks() -> None: + failures: Final = _failures(STALE_HEAD, changed_files=CODE_ONLY, bot=True) + assert [failure for failure in failures if failure.startswith(guard.BACKUP_PATH)] + assert [failure for failure in failures if failure.startswith(guard.SCHEMA_PATH)] + + +def test_contract_names_the_skip() -> None: + assert guard.contract_for(False, CODE_ONLY) == "human PR, cost map untouched" + assert guard.contract_for(False, (*CODE_ONLY, guard.SCHEMA_PATH)) == "human PR, file checks only" + assert guard.contract_for(True, CODE_ONLY) == "bot contract enforced" + + def test_bot_may_only_touch_the_cost_map_files() -> None: changed = (*guard.GUARDED_PATHS, "litellm/utils.py", ".github/workflows/cost-map-guard.yml") assert _failures(BASE, changed_files=changed, bot=False) == () @@ -147,6 +181,16 @@ def _commit(repo: Path, cost_map: dict[str, object], message: str) -> str: (repo / guard.BACKUP_PATH).parent.mkdir(exist_ok=True) (repo / guard.BACKUP_PATH).write_text(text) (repo / guard.SCHEMA_PATH).write_text(schema_module.render(schema_module.build_schema(cost_map))) + return _git_commit(repo, message) + + +def _commit_code_only(repo: Path, message: str) -> str: + (repo / "litellm").mkdir(exist_ok=True) + (repo / "litellm" / "utils.py").write_text(f"print('{message}')\n") + return _git_commit(repo, message) + + +def _git_commit(repo: Path, message: str) -> str: subprocess.run(("git", "add", "-A"), cwd=repo, check=True) subprocess.run( ("git", "-c", "user.name=t", "-c", "user.email=t@example.com", "commit", "-q", "-m", message), @@ -186,6 +230,40 @@ def test_main_reads_both_revisions_from_git( assert expected_line in result.stdout.splitlines() +def test_main_skips_the_file_checks_on_a_stale_base_the_pr_never_touched(tmp_path: Path) -> None: + subprocess.run(("git", "init", "-q", str(tmp_path)), check=True) + _commit(tmp_path, BASE_MAP, "base") + (tmp_path / guard.BACKUP_PATH).write_text(_serialize({**BASE_MAP, "openrouter/b": _entry(3e-06)})) + (tmp_path / guard.SCHEMA_PATH).write_text("{}") + stale_base: Final = _commit_code_only(tmp_path, "stale base with drifted backup and schema") + head: Final = _commit_code_only(tmp_path, "code change on the stale base") + human: Final = _run_guard(tmp_path, stale_base, head, "litellm_fix_pricing") + assert human.returncode == 0, human.stdout + human.stderr + assert "cost map guard passed (human PR, cost map untouched)" in human.stdout.splitlines() + bot: Final = _run_guard(tmp_path, stale_base, head, BOT_REF) + assert bot.returncode == 1 + backup_failure: Final = f"- {guard.BACKUP_PATH} differs from {guard.COST_MAP_PATH}; copy the root file over it" + assert backup_failure in bot.stdout.splitlines() + + +def test_main_keeps_the_file_checks_when_a_cost_map_file_is_renamed(tmp_path: Path) -> None: + subprocess.run(("git", "init", "-q", str(tmp_path)), check=True) + base: Final = _commit(tmp_path, BASE_MAP, "base") + subprocess.run(("git", "mv", guard.COST_MAP_PATH, "renamed.json"), cwd=tmp_path, check=True) + head: Final = _git_commit(tmp_path, "rename the cost map") + result: Final = _run_guard(tmp_path, base, head, "litellm_fix_pricing") + assert result.returncode == 1, result.stdout + result.stderr + assert "cost map guard failed (human PR, file checks only):" in result.stdout.splitlines() + + +def test_main_fails_when_the_changed_files_cannot_be_read(tmp_path: Path) -> None: + subprocess.run(("git", "init", "-q", str(tmp_path)), check=True) + head: Final = _commit(tmp_path, BASE_MAP, "head") + result: Final = _run_guard(tmp_path, "0" * 40, head, "litellm_fix_pricing") + assert result.returncode == 1, result.stdout + result.stderr + assert result.stdout.startswith("cost map guard failed: git diff ") + + def test_main_rejects_a_bot_pr_that_edits_code(tmp_path: Path) -> None: subprocess.run(("git", "init", "-q", str(tmp_path)), check=True) base = _commit(tmp_path, BASE_MAP, "base")