From b8fb50f70e82f5eed8a8d56cdb5b6b1ca4e1e7b7 Mon Sep 17 00:00:00 2001
From: Kerry Lu <kerry@berri.ai>
Date: Wed, 28 Oct 2026 15:03:40 -0700
Subject: [PATCH] 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>
---
 ci_cd/spend_logs_guard.py           | 11 ++++++++---
 tests/unit/test_spend_logs_guard.py | 23 +++++++++++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/ci_cd/spend_logs_guard.py b/ci_cd/spend_logs_guard.py
index 64e33c4fb70..fac9d4ba5d2 100644
--- a/ci_cd/spend_logs_guard.py
+++ b/ci_cd/spend_logs_guard.py
@@ -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"
 
 
diff --git a/tests/unit/test_spend_logs_guard.py b/tests/unit/test_spend_logs_guard.py
index 6a3355e1c52..9ac2c923632 100644
--- a/tests/unit/test_spend_logs_guard.py
+++ b/tests/unit/test_spend_logs_guard.py
@@ -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