fix(ci): keep the spend logs guard approval through a later COMMENTED review (LIT-10242) (#48901)

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Kerry Lu 2026-10-28 15:03:40 -07:00 • committed by GitHub
parent 5215c2d3f7
commit b8fb50f70e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 31 additions and 3 deletions

View file

@ -27,7 +27,8 @@ WATCHED: Final = (
"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)
TABLE: Final = re.compile(r"(?<![A-Za-z0-9])LiteLLM_SpendLogs(?![A-Za-z0-9])", re.IGNORECASE)
DECIDING_STATES: Final = frozenset({"APPROVED", "CHANGES_REQUESTED", "DISMISSED"})
class Reviewer(BaseModel):
@ -73,8 +74,12 @@ def _path_of(section: str) -> str:
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)
"""True when `login`'s most recent review that changes GitHub's approval state is an approval.
COMMENTED and PENDING reviews leave an earlier approval in force on GitHub, so they are skipped here too.
"""
own: Final = tuple(review.state for review in reviews if review.user.login == login)
states: Final = tuple(state for state in own if state in DECIDING_STATES)
return bool(states) and states[-1] == "APPROVED"

View file

@ -3,6 +3,8 @@ import sys
from pathlib import Path
from typing import Final
import pytest
_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
@ -35,6 +37,13 @@ class TestSpendLogHits:
elsewhere: Final = _diff("litellm/proxy/utils.py", 'table = "LiteLLM_SpendLogs"', 5)
assert guard.spend_log_hits(removed + other + elsewhere) == ()
def test_a_table_whose_name_only_starts_with_the_watched_one_is_not_a_hit(self):
archive: Final = _diff("schema.prisma", "model LiteLLM_SpendLogsArchive {", 900)
index: Final = _diff("schema.prisma", " @@index([team_id], map: \"LiteLLM_SpendLogs_team_id_idx\")", 760)
assert guard.spend_log_hits(archive + index) == (
'schema.prisma: @@index([team_id], map: "LiteLLM_SpendLogs_team_id_idx")',
)
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
@ -59,6 +68,20 @@ class TestApprovedBy:
assert guard.approved_by(reviews, "kerry-berri") is True
assert guard.approved_by(reviews[::-1], "kerry-berri") is False
@pytest.mark.parametrize(
("states", "approved"),
(
(("APPROVED", "COMMENTED"), True),
(("APPROVED", "PENDING"), True),
(("CHANGES_REQUESTED", "COMMENTED"), False),
(("APPROVED", "DISMISSED"), False),
(("COMMENTED",), False),
),
)
def test_a_comment_or_pending_review_does_not_change_the_answer(self, states, approved):
reviews: Final = tuple(guard.Review(state=state, user=guard.Reviewer(login="kerry-berri")) for state in states)
assert guard.approved_by(reviews, "kerry-berri") is approved
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