ci: spend-logs-guard requires kerry-berri approval on LiteLLM_SpendLogs schema and migration changes (#48518)

* ci: spend-logs-guard requires kerry-berri approval on LiteLLM_SpendLogs schema and migration changes

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* ci(spend-logs-guard): flatten paginated reviews with jq, check out the base ref

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* ci(spend-logs-guard): read the diff from the pulls/files API instead of fetching PR commits

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* Revert "ci(spend-logs-guard): read the diff from the pulls/files API instead of fetching PR commits"

This reverts commit 36da4fe034.

---------

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

.github/workflows/spend-logs-guard.yml

@@ -0,0 +1,49 @@
+name: Spend logs guard
+
+on: # zizmor: ignore[dangerous-triggers] runs the base branch's code only; the PR's diff is read as data and never executed
+  pull_request_target:
+    branches:
+      - main
+  pull_request_review:
+    types:
+      - submitted
+      - dismissed
+
+permissions:
+  contents: read
+  pull-requests: read
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  spend-logs-guard:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
+        with:
+          ref: ${{ github.event.pull_request.base.ref }}
+          persist-credentials: false
+      - name: Fetch the pull request head, its merge base and its reviews
+        id: revisions
+        env:
+          GH_TOKEN: ${{ github.token }}
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+        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"
+          gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate --slurp | jq 'add // []' > "$RUNNER_TEMP/reviews.json"
+          echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT"
+      - name: Set up uv
+        uses: ./.github/actions/setup-uv-with-retries
+        with:
+          version: "0.10.9"
+      - name: Run the guard
+        env:
+          MERGE_BASE: ${{ steps.revisions.outputs.merge_base }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        run: |
+          uv run --frozen python ci_cd/spend_logs_guard.py --base "$MERGE_BASE" --head "$HEAD_SHA" --approver kerry-berri --reviews "$RUNNER_TEMP/reviews.json"

ci_cd/spend_logs_guard.py

@@ -0,0 +1,119 @@
+"""Required check: a pull request that touches LiteLLM_SpendLogs in schema.prisma or a migration
+needs an approving review from the spend-log owner before it can merge.
+
+CODEOWNERS can only match file paths, and every schema change edits the same schema.prisma copies,
+so a path rule would put every migration on the owner. This gate reads the diff instead: when no
+added line in the watched files mentions the table it passes at once, otherwise it passes only once
+the owner's latest review on the pull request is an approval. The workflow re-runs on every review.
+"""
+
+from __future__ import annotations
+
+import argparse
+import re
+import subprocess
+import sys
+from collections.abc import Sequence
+from pathlib import Path
+from typing import Final
+
+from pydantic import BaseModel, ConfigDict, TypeAdapter
+
+WATCHED: Final = (
+    "schema.prisma",
+    "litellm/proxy/schema.prisma",
+    "litellm-proxy-extras/litellm_proxy_extras/schema.prisma",
+)
+MIGRATIONS_PREFIX: Final = "litellm-proxy-extras/litellm_proxy_extras/migrations/"
+TABLE: Final = re.compile(r"(?<![A-Za-z0-9])LiteLLM_SpendLogs", re.IGNORECASE)
+
+
+class Reviewer(BaseModel):
+    model_config = ConfigDict(frozen=True, extra="ignore")
+    login: str
+
+
+class Review(BaseModel):
+    model_config = ConfigDict(frozen=True, extra="ignore")
+    state: str
+    user: Reviewer
+
+
+_REVIEWS: Final = TypeAdapter(list[Review])
+
+
+def watched(path: str) -> bool:
+    return path in WATCHED or (path.startswith(MIGRATIONS_PREFIX) and path.endswith("/migration.sql"))
+
+
+def _added_lines(section: str) -> tuple[str, ...]:
+    body: Final = section.split("\n@@", 1)
+    return tuple(line[1:] for line in body[1].splitlines() if line.startswith("+")) if len(body) == 2 else ()
+
+
+def spend_log_hits(diff: str) -> tuple[str, ...]:
+    """`path: added line` for every added line in a watched file that names the table."""
+    sections: Final = re.split(r"^diff --git [^\n]*\n", diff, flags=re.MULTILINE)
+    return tuple(f"{path}: {line.strip()}" for path, line in _watched_added_lines(sections) if TABLE.search(line))
+
+
+def _watched_added_lines(sections: Sequence[str]) -> tuple[tuple[str, str], ...]:
+    pairs: Final = tuple((_path_of(section), section) for section in sections)
+    return tuple(
+        (path, line) for path, section in pairs if watched(path) for line in _added_lines(section)
+    )  # comprehension-ok: path then its lines
+
+
+def _path_of(section: str) -> str:
+    match: Final = re.search(r"^\+\+\+ b/(.+)$", section, flags=re.MULTILINE)
+    return match.group(1) if match else ""
+
+
+def approved_by(reviews: Sequence[Review], login: str) -> bool:
+    """True when `login`'s most recent review on the pull request is an approval."""
+    states: Final = tuple(review.state for review in reviews if review.user.login == login)
+    return bool(states) and states[-1] == "APPROVED"
+
+
+def added_diff(base: str, head: str) -> str:
+    result: Final = subprocess.run(
+        ("git", "diff", "--unified=0", "--no-color", f"{base}..{head}", "--", *WATCHED, MIGRATIONS_PREFIX),
+        check=True,
+        capture_output=True,
+        text=True,
+    )
+    return result.stdout
+
+
+class Arguments(BaseModel):
+    model_config = ConfigDict(frozen=True)
+    base: str
+    head: str
+    approver: str
+    reviews: Path
+
+
+def main(argv: Sequence[str]) -> int:
+    parser: Final = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--base", required=True, help="merge base of the pull request")
+    parser.add_argument("--head", required=True, help="head sha of the pull request")
+    parser.add_argument("--approver", required=True, help="GitHub login whose approval unlocks the merge")
+    parser.add_argument("--reviews", required=True, help="JSON file with the pull request's reviews")
+    args: Final = Arguments.model_validate(vars(parser.parse_args(argv)))
+    hits: Final = spend_log_hits(added_diff(args.base, args.head))
+    if not hits:
+        sys.stdout.write("No added line in schema.prisma or a migration names LiteLLM_SpendLogs; nothing to review.\n")
+        return 0
+    if approved_by(_REVIEWS.validate_json(args.reviews.read_bytes()), args.approver):
+        sys.stdout.write(f"LiteLLM_SpendLogs change approved by {args.approver}: {', '.join(hits)}\n")
+        return 0
+    listed: Final = "".join(f"  {hit}\n" for hit in hits)
+    sys.stdout.write(
+        f"This pull request changes LiteLLM_SpendLogs and needs an approving review from @{args.approver}:\n"
+        f"{listed}The check re-runs when a review is submitted.\n"
+    )
+    return 1
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))

tests/unit/test_spend_logs_guard.py

@@ -0,0 +1,79 @@
+import importlib.util
+import sys
+from pathlib import Path
+from typing import Final
+
+_SCRIPT: Final = Path(__file__).resolve().parents[2] / "ci_cd" / "spend_logs_guard.py"
+_SPEC: Final = importlib.util.spec_from_file_location("spend_logs_guard", _SCRIPT)
+assert _SPEC is not None and _SPEC.loader is not None
+guard: Final = importlib.util.module_from_spec(_SPEC)
+sys.modules["spend_logs_guard"] = guard
+_SPEC.loader.exec_module(guard)
+
+
+def _diff(path: str, added: str, start: int = 10) -> str:
+    return f"diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -{start},0 +{start},1 @@\n+{added}\n"
+
+
+class TestSpendLogHits:
+    def test_an_added_schema_line_naming_the_table_is_a_hit(self):
+        diff: Final = _diff(
+            "schema.prisma", '  @@index([api_key, startTime], map: "LiteLLM_SpendLogs_api_key_startTime_idx")', 740
+        )
+        assert guard.spend_log_hits(diff) == (
+            'schema.prisma: @@index([api_key, startTime], map: "LiteLLM_SpendLogs_api_key_startTime_idx")',
+        )
+
+    def test_an_added_migration_line_is_a_hit_in_any_case(self):
+        path: Final = "litellm-proxy-extras/litellm_proxy_extras/migrations/20260101000000_x/migration.sql"
+        hits: Final = guard.spend_log_hits(_diff(path, 'CREATE INDEX "i" ON "litellm_spendlogs"("api_key");', 3))
+        assert hits == (f'{path}: CREATE INDEX "i" ON "litellm_spendlogs"("api_key");',)
+
+    def test_a_removed_line_or_another_table_or_an_unwatched_file_is_not_a_hit(self):
+        removed: Final = "--- a/schema.prisma\n+++ b/schema.prisma\n@@ -1,1 +1,0 @@\n-model LiteLLM_SpendLogs {\n"
+        other: Final = _diff("schema.prisma", "  team_id String  // LiteLLM_TeamTable, not MyLiteLLM_SpendLogs", 5)
+        elsewhere: Final = _diff("litellm/proxy/utils.py", 'table = "LiteLLM_SpendLogs"', 5)
+        assert guard.spend_log_hits(removed + other + elsewhere) == ()
+
+    def test_two_files_in_one_diff_are_attributed_to_their_own_path(self):
+        diff: Final = _diff("litellm/proxy/schema.prisma", "  a LiteLLM_SpendLogs", 1) + _diff(
+            "schema.prisma", "  b String", 1
+        )
+        assert guard.spend_log_hits(diff) == ("litellm/proxy/schema.prisma: a LiteLLM_SpendLogs",)
+
+
+class TestApprovedBy:
+    def test_the_latest_review_of_the_approver_decides(self):
+        reviews: Final = tuple(
+            guard.Review(state=state, user=guard.Reviewer(login=login))
+            for login, state in (
+                ("kerry-berri", "CHANGES_REQUESTED"),
+                ("someone-else", "APPROVED"),
+                ("kerry-berri", "APPROVED"),
+            )
+        )
+        assert guard.approved_by(reviews, "kerry-berri") is True
+        assert guard.approved_by(reviews[::-1], "kerry-berri") is False
+
+    def test_no_review_from_the_approver_is_not_an_approval(self):
+        other: Final = guard.Review(state="APPROVED", user=guard.Reviewer(login="someone-else"))
+        assert guard.approved_by((other,), "kerry-berri") is False
+        assert guard.approved_by((), "kerry-berri") is False
+
+
+class TestExitCode:
+    def test_a_pull_request_without_spend_log_lines_passes_without_reviews(self, tmp_path, monkeypatch, capsys):
+        monkeypatch.setattr(guard, "added_diff", lambda base, head: _diff("schema.prisma", "  foo String", 1))
+        reviews: Final = tmp_path / "r.json"
+        reviews.write_text("[]")
+        assert guard.main(["--base", "a", "--head", "b", "--approver", "kerry-berri", "--reviews", str(reviews)]) == 0
+        assert "nothing to review" in capsys.readouterr().out
+
+    def test_a_spend_log_change_fails_until_the_approver_approves(self, tmp_path, monkeypatch, capsys):
+        monkeypatch.setattr(guard, "added_diff", lambda base, head: _diff("schema.prisma", "  x LiteLLM_SpendLogs", 7))
+        reviews: Final = tmp_path / "r.json"
+        reviews.write_text('[{"user": {"login": "someone-else"}, "state": "APPROVED"}]')
+        assert guard.main(["--base", "a", "--head", "b", "--approver", "kerry-berri", "--reviews", str(reviews)]) == 1
+        assert "schema.prisma: x LiteLLM_SpendLogs" in capsys.readouterr().out
+        reviews.write_text('[{"user": {"login": "kerry-berri"}, "state": "APPROVED"}]')
+        assert guard.main(["--base", "a", "--head", "b", "--approver", "kerry-berri", "--reviews", str(reviews)]) == 0