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

@@ -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))

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")