ci: request perf review when a PR adds prisma queries or edits a schema

Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>

.github/workflows/notify-perf-on-db-queries.yml

@@ -0,0 +1,95 @@
+name: Notify Perf On DB Queries
+
+on:
+  pull_request:
+    branches:
+      - main
+      - litellm_internal_staging
+      - litellm_oss_staging
+      - "litellm_**"
+
+permissions: {}
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  notify-perf:
+    name: notify-perf
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    permissions:
+      contents: read
+      pull-requests: write
+    steps:
+      - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+          fetch-depth: 1
+          persist-credentials: false
+
+      - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+        with:
+          python-version: "3.12"
+
+      - name: Install detector dependencies
+        run: pip install --no-cache-dir "pydantic==2.13.4"
+
+      - name: Detect added DB queries
+        id: detect
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+          REPO: ${{ github.repository }}
+        run: |
+          gh api "repos/${REPO}/pulls/${PR_NUMBER}/files" --paginate \
+            --jq '[.[] | {filename, patch}]' > changed_files.json
+          if python scripts/detect_new_db_queries.py < changed_files.json > findings.txt; then
+            echo "found=true" >> "$GITHUB_OUTPUT"
+            {
+              echo "### New DB access in this PR"
+              echo
+              echo '```'
+              cat findings.txt
+              echo '```'
+            } >> "$GITHUB_STEP_SUMMARY"
+          else
+            echo "found=false" >> "$GITHUB_OUTPUT"
+          fi
+
+      - name: Request perf review
+        if: steps.detect.outputs.found == 'true' && github.event.pull_request.head.repo.fork == false
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+          REPO: ${{ github.repository }}
+          PERF_REVIEWER: yassin-berriai
+          PR_AUTHOR: ${{ github.event.pull_request.user.login }}
+        run: |
+          if [ "$PR_AUTHOR" = "$PERF_REVIEWER" ]; then
+            echo "PR author is the perf reviewer; nothing to request."
+            exit 0
+          fi
+          reviewed=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/reviews" --paginate \
+            --jq "[.[] | select(.user.login == \"${PERF_REVIEWER}\")] | length")
+          if [ "$reviewed" != "0" ]; then
+            echo "${PERF_REVIEWER} already reviewed this PR."
+            exit 0
+          fi
+          gh api --method POST "repos/${REPO}/pulls/${PR_NUMBER}/requested_reviewers" \
+            -f "reviewers[]=${PERF_REVIEWER}" >/dev/null
+          marker="<!-- notify-perf-on-db-queries -->"
+          existing=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" --paginate \
+            --jq "[.[] | select(.body | contains(\"${marker}\"))] | length")
+          if [ "$existing" != "0" ]; then
+            echo "Perf review comment already posted."
+            exit 0
+          fi
+          {
+            echo "$marker"
+            echo "This PR adds Prisma queries or changes a Prisma schema, so @${PERF_REVIEWER} has been requested for a performance review."
+            echo
+            echo "The matched lines are in the \`notify-perf\` job summary. Reminder: dashboards and management endpoints should read the daily aggregate tables, since a query against \`LiteLLM_SpendLogs\` scales with request volume and a date-bounded window is not a bounded amount of work."
+          } > perf_comment.md
+          gh pr comment "${PR_NUMBER}" --repo "${REPO}" --body-file perf_comment.md

scripts/detect_new_db_queries.py

@@ -0,0 +1,65 @@
+import json
+import re
+import sys
+from dataclasses import dataclass
+from pathlib import PurePosixPath
+
+from pydantic import BaseModel, TypeAdapter
+
+RAW_SQL_CALL = re.compile(r"\b(?:query_raw|query_first|execute_raw)\b")
+PRISMA_MODEL_CALL = re.compile(
+    r"\.db\.[A-Za-z_][A-Za-z0-9_]*\.(?:find_many|find_first|find_unique|find_unique_or_raise|count|aggregate|group_by|create_many|update_many|delete_many)\b"
+)
+SCHEMA_SUFFIX = ".prisma"
+IGNORED_PREFIXES = ("tests/", "ui/", "docs/", "litellm/proxy/_experimental/out/")
+
+
+class ChangedFile(BaseModel):
+    filename: str
+    patch: str | None = None
+
+
+CHANGED_FILES = TypeAdapter(tuple[ChangedFile, ...])
+
+
+@dataclass(frozen=True, slots=True)
+class Finding:
+    path: str
+    detail: str
+
+
+def _added_lines(patch: str) -> tuple[str, ...]:
+    return tuple(line[1:] for line in patch.splitlines() if line.startswith("+") and not line.startswith("+++"))
+
+
+def _is_ignored(path: str) -> bool:
+    return path.startswith(IGNORED_PREFIXES)
+
+
+def findings_for_file(changed: ChangedFile) -> tuple[Finding, ...]:
+    if _is_ignored(changed.filename):
+        return ()
+    if PurePosixPath(changed.filename).suffix == SCHEMA_SUFFIX:
+        return (Finding(path=changed.filename, detail="prisma schema changed"),)
+    if changed.patch is None or PurePosixPath(changed.filename).suffix != ".py":
+        return ()
+    return tuple(
+        Finding(path=changed.filename, detail=line.strip())
+        for line in _added_lines(changed.patch)
+        if RAW_SQL_CALL.search(line) or PRISMA_MODEL_CALL.search(line)
+    )
+
+
+def detect(files: tuple[ChangedFile, ...]) -> tuple[Finding, ...]:
+    return tuple(finding for changed in files for finding in findings_for_file(changed))
+
+
+def main() -> int:
+    findings = detect(CHANGED_FILES.validate_python(json.load(sys.stdin)))
+    for finding in findings:
+        print(f"{finding.path}: {finding.detail}")
+    return 0 if findings else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_litellm/test_detect_new_db_queries.py

@@ -0,0 +1,102 @@
+"""Tests for scripts/detect_new_db_queries.py.
+
+The detector's contract is "a PR that adds DB access is flagged": raw SQL and Prisma
+model calls on added lines count, removed lines and tests/UI paths do not, and a
+schema edit counts on its own. The regression that motivated it is #33978, whose
+`prisma_client.db.query_raw` against LiteLLM_SpendLogs shipped without a perf review.
+"""
+
+import importlib.util
+from pathlib import Path
+
+_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "detect_new_db_queries.py"
+_spec = importlib.util.spec_from_file_location("detect_new_db_queries", _MODULE_PATH)
+detector = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(detector)
+
+
+def _changed(filename: str, patch: str | None):
+    return detector.ChangedFile(filename=filename, patch=patch)
+
+
+def test_flags_added_raw_sql_query():
+    findings = detector.detect(
+        (
+            _changed(
+                "litellm/proxy/management_endpoints/tool_management_endpoints.py",
+                "@@ -1,0 +1,2 @@\n+    rows = await prisma_client.db.query_raw(\n+        'SELECT 1'\n",
+            ),
+        )
+    )
+    assert [f.detail for f in findings] == ["rows = await prisma_client.db.query_raw("]
+
+
+def test_flags_added_prisma_model_call():
+    findings = detector.detect(
+        (
+            _changed(
+                "litellm/proxy/spend_tracking/spend_management_endpoints.py",
+                '@@ -1,0 +1,1 @@\n+    rows = await prisma_client.db.litellm_spendlogs.find_many(where={"a": 1})\n',
+            ),
+        )
+    )
+    assert len(findings) == 1
+
+
+def test_ignores_removed_and_context_lines():
+    findings = detector.detect(
+        (
+            _changed(
+                "litellm/proxy/db/db_spend_update_writer.py",
+                "@@ -1,3 +1,1 @@\n-    await prisma_client.db.query_raw('SELECT 1')\n     await unrelated()\n",
+            ),
+        )
+    )
+    assert findings == ()
+
+
+def test_ignores_tests_and_ui_paths():
+    patch = "@@ -1,0 +1,1 @@\n+    await prisma_client.db.query_raw('SELECT 1')\n"
+    findings = detector.detect(
+        (
+            _changed("tests/test_litellm/test_something.py", patch),
+            _changed("ui/litellm-dashboard/src/thing.py", patch),
+        )
+    )
+    assert findings == ()
+
+
+def test_flags_schema_change_without_a_patch():
+    findings = detector.detect((_changed("litellm/proxy/schema.prisma", None),))
+    assert [f.detail for f in findings] == ["prisma schema changed"]
+
+
+def test_ignores_unrelated_python_change():
+    findings = detector.detect(
+        (
+            _changed(
+                "litellm/main.py",
+                "@@ -1,0 +1,1 @@\n+    response = await client.chat.completions.create(**kwargs)\n",
+            ),
+            _changed("README.md", "@@ -1,0 +1,1 @@\n+docs\n"),
+        )
+    )
+    assert findings == ()
+
+
+def test_main_exit_codes_signal_whether_anything_was_found(monkeypatch, capsys, tmp_path):
+    flagged = tmp_path / "flagged.json"
+    flagged.write_text(
+        '[{"filename": "litellm/proxy/db/x.py",'
+        ' "patch": "@@ -1,0 +1,1 @@\\n+    await prisma_client.db.query_raw(sql)\\n"}]'
+    )
+    with flagged.open() as handle:
+        monkeypatch.setattr(detector.sys, "stdin", handle)
+        assert detector.main() == 0
+    assert "litellm/proxy/db/x.py" in capsys.readouterr().out
+
+    clean = tmp_path / "clean.json"
+    clean.write_text('[{"filename": "README.md", "patch": "@@ -1,0 +1,1 @@\\n+docs\\n"}]')
+    with clean.open() as handle:
+        monkeypatch.setattr(detector.sys, "stdin", handle)
+        assert detector.main() == 1