mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
ci: gate PR bodies on the template's HTML comment rules
This commit is contained in:
parent
b0fac57fe4
commit
5d9f28c48c
3 changed files with 312 additions and 0 deletions
154
.github/scripts/check_pr_body.py
vendored
Normal file
154
.github/scripts/check_pr_body.py
vendored
Normal file
|
|
@ -0,0 +1,154 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Gate PR bodies on the machine-checkable rules that live inside HTML comments in
|
||||
.github/pull_request_template.md, since those comments are invisible in rendered PRs
|
||||
and are stripped from the template copies that agent harnesses inject into context."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
from dataclasses import dataclass
|
||||
from functools import reduce
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
BULLET_WORD_TARGET: Final = 10
|
||||
BULLET_WORD_CAP: Final = 14
|
||||
BULLET_SECTIONS: Final = ("tldr", "caveats (if any)")
|
||||
QA_RUNBOOK_SECTION: Final = "qa runbook"
|
||||
E2E_PREFIX: Final = "tests/e2e/"
|
||||
PLACEHOLDER_TOKENS: Final = ("<blah>",)
|
||||
NO_CAVEATS_PATTERN: Final = re.compile(r"^(none|n/?a)[.!]?$", re.IGNORECASE)
|
||||
HTML_COMMENT_PATTERN: Final = re.compile(r"<!--.*?-->", re.DOTALL)
|
||||
HEADING_PATTERN: Final = re.compile(r"^#{2,6}\s+(?P<title>.+?)\s*$")
|
||||
BULLET_PATTERN: Final = re.compile(r"^\s*(?:[-*+]|\d+[.)])\s+(?P<text>.*)$")
|
||||
LABEL_PATTERN: Final = re.compile(r"^[^-*+].*:\s*$")
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class Violation:
|
||||
section: str
|
||||
detail: str
|
||||
|
||||
def __str__(self) -> str:
|
||||
return f"{self.section}: {self.detail}"
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class Section:
|
||||
title: str
|
||||
lines: tuple[str, ...]
|
||||
|
||||
|
||||
def strip_html_comments(body: str) -> str:
|
||||
return HTML_COMMENT_PATTERN.sub("", body.replace("\r\n", "\n"))
|
||||
|
||||
|
||||
def split_sections(body: str) -> tuple[Section, ...]:
|
||||
lines: Final = tuple(body.split("\n"))
|
||||
headings: Final = tuple(
|
||||
(index, match.group("title")) for index, line in enumerate(lines) if (match := HEADING_PATTERN.match(line))
|
||||
)
|
||||
ends: Final = tuple(index for index, _ in headings[1:]) + (len(lines),)
|
||||
return tuple(Section(title=title, lines=lines[start + 1 : end]) for (start, title), end in zip(headings, ends))
|
||||
|
||||
|
||||
def group_bullets(lines: tuple[str, ...]) -> tuple[tuple[str, ...], tuple[str, ...]]:
|
||||
def classify(acc: tuple[tuple[str, ...], tuple[str, ...]], line: str) -> tuple[tuple[str, ...], tuple[str, ...]]:
|
||||
bullets, prose = acc
|
||||
stripped: Final = line.strip()
|
||||
if not stripped:
|
||||
return acc
|
||||
bullet_match: Final = BULLET_PATTERN.match(line)
|
||||
if bullet_match:
|
||||
return (*bullets, bullet_match.group("text").strip()), prose
|
||||
if bullets and line[:1].isspace():
|
||||
return (*bullets[:-1], f"{bullets[-1]} {stripped}"), prose
|
||||
return bullets, (*prose, stripped)
|
||||
|
||||
return reduce(classify, lines, ((), ()))
|
||||
|
||||
|
||||
def check_bullet_section(section: Section) -> tuple[Violation, ...]:
|
||||
bullets, prose = group_bullets(section.lines)
|
||||
prose_violations: Final = tuple(
|
||||
Violation(section.title, f'prose line "{line[:60]}" must be a short bullet instead')
|
||||
for line in prose
|
||||
if not LABEL_PATTERN.match(line) and not NO_CAVEATS_PATTERN.match(line)
|
||||
)
|
||||
length_violations: Final = tuple(
|
||||
Violation(
|
||||
section.title,
|
||||
f'bullet "{bullet[:60]}" has {len(bullet.split())} words;'
|
||||
f" keep bullets to roughly {BULLET_WORD_TARGET} words ({BULLET_WORD_CAP} max)",
|
||||
)
|
||||
for bullet in bullets
|
||||
if len(bullet.split()) > BULLET_WORD_CAP
|
||||
)
|
||||
return prose_violations + length_violations
|
||||
|
||||
|
||||
def is_placeholder_line(line: str) -> bool:
|
||||
bullet_match: Final = BULLET_PATTERN.match(line)
|
||||
content: Final = bullet_match.group("text").strip() if bullet_match else line.strip()
|
||||
return content == "..." or any(token in line for token in PLACEHOLDER_TOKENS)
|
||||
|
||||
|
||||
def check_placeholders(sections: tuple[Section, ...]) -> tuple[Violation, ...]:
|
||||
return tuple(
|
||||
Violation(section.title, f'template placeholder left in: "{line.strip()[:60]}"')
|
||||
for section in sections
|
||||
for line in section.lines
|
||||
if is_placeholder_line(line)
|
||||
)
|
||||
|
||||
|
||||
def check_qa_runbook(sections: tuple[Section, ...], changed_files: tuple[str, ...]) -> tuple[Violation, ...]:
|
||||
has_runbook: Final = any(section.title.lower() == QA_RUNBOOK_SECTION for section in sections)
|
||||
touches_e2e: Final = any(path.startswith(E2E_PREFIX) for path in changed_files)
|
||||
if has_runbook and not touches_e2e:
|
||||
return (
|
||||
Violation(
|
||||
"QA runbook",
|
||||
"delete this section; the template only wants it when the PR edits tests/e2e",
|
||||
),
|
||||
)
|
||||
return ()
|
||||
|
||||
|
||||
def check_body(body: str, changed_files: tuple[str, ...]) -> tuple[Violation, ...]:
|
||||
sections: Final = split_sections(strip_html_comments(body))
|
||||
bullet_violations: Final = tuple(
|
||||
violation
|
||||
for section in sections
|
||||
if section.title.lower() in BULLET_SECTIONS
|
||||
for violation in check_bullet_section(section)
|
||||
)
|
||||
return bullet_violations + check_placeholders(sections) + check_qa_runbook(sections, changed_files)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser: Final = argparse.ArgumentParser()
|
||||
parser.add_argument("--changed-files", type=Path, required=True)
|
||||
args: Final = parser.parse_args()
|
||||
body: Final = os.environ.get("PR_BODY", "")
|
||||
changed_files: Final = tuple(
|
||||
line.strip() for line in args.changed_files.read_text(encoding="utf-8").splitlines() if line.strip()
|
||||
)
|
||||
violations: Final = check_body(body, changed_files)
|
||||
for violation in violations:
|
||||
print(f"::error title=PR body template::{violation}")
|
||||
if violations:
|
||||
print(
|
||||
"\nThe rules above come from the HTML comments inside .github/pull_request_template.md;"
|
||||
" open that file to see every rule next to its section."
|
||||
)
|
||||
return 1
|
||||
print("PR body follows the template comment rules.")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
47
.github/workflows/pr-body-template.yml
vendored
Normal file
47
.github/workflows/pr-body-template.yml
vendored
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
name: PR Body Template
|
||||
|
||||
# The imperative rules in .github/pull_request_template.md live inside HTML
|
||||
# comments, which are invisible in the rendered PR and get stripped from the
|
||||
# template copies that agent harnesses inject into context, so authors (human
|
||||
# and AI) routinely miss them. This workflow turns the machine-checkable
|
||||
# subset into a real gate: TLDR and Caveats must be short bullets, no template
|
||||
# placeholders may remain, and the QA runbook section must be deleted unless
|
||||
# the PR edits tests/e2e.
|
||||
#
|
||||
# Skip with the ignore-pr-body-template label (mirrors conventional-commits).
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, edited, reopened, synchronize, labeled, unlabeled]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
|
||||
concurrency:
|
||||
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}
|
||||
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
|
||||
|
||||
jobs:
|
||||
lint-pr-body:
|
||||
name: Validate PR body
|
||||
if: ${{ !endsWith(github.event.pull_request.user.login, '[bot]') && !contains(github.event.pull_request.labels.*.name, 'ignore-pr-body-template') }}
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout lint script
|
||||
uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
|
||||
with:
|
||||
sparse-checkout: .github/scripts
|
||||
persist-credentials: false
|
||||
|
||||
- name: List changed files
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
run: gh api "repos/${REPO}/pulls/${PR_NUMBER}/files" --paginate --jq '.[].filename' > changed_files.txt
|
||||
|
||||
- name: Check body against template comment rules
|
||||
env:
|
||||
PR_BODY: ${{ github.event.pull_request.body }}
|
||||
run: python3 .github/scripts/check_pr_body.py --changed-files changed_files.txt
|
||||
111
tests/test_litellm/test_github_check_pr_body.py
Normal file
111
tests/test_litellm/test_github_check_pr_body.py
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
"""Unit tests for `.github/scripts/check_pr_body.py`."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
SCRIPT_PATH = Path(__file__).resolve().parents[2] / ".github" / "scripts" / "check_pr_body.py"
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def checker():
|
||||
spec = importlib.util.spec_from_file_location("check_pr_body", SCRIPT_PATH)
|
||||
assert spec and spec.loader, f"Could not load spec for {SCRIPT_PATH}"
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
sys.modules["check_pr_body"] = module
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
COMPLIANT_BODY = """## TLDR
|
||||
|
||||
Problem this solves:
|
||||
|
||||
- Any counts keep drifting toward their ceilings
|
||||
|
||||
How it solves it:
|
||||
|
||||
- Real types at each Any source across 56 files
|
||||
|
||||
## Caveats (if any)
|
||||
|
||||
- Rate limiter no longer crashes on usage-less responses
|
||||
|
||||
### Final Attestation
|
||||
|
||||
- [ ] The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
|
||||
"""
|
||||
|
||||
|
||||
def test_compliant_body_passes(checker):
|
||||
assert checker.check_body(COMPLIANT_BODY, ("litellm/main.py",)) == ()
|
||||
|
||||
|
||||
def test_prose_caveats_fails(checker):
|
||||
body = "## Caveats (if any)\n\nThree micro-hardenings ride along with the typing because honest annotation exposed them.\n"
|
||||
violations = checker.check_body(body, ())
|
||||
assert len(violations) == 1
|
||||
assert "must be a short bullet" in violations[0].detail
|
||||
|
||||
|
||||
def test_overlong_bullet_fails(checker):
|
||||
bullet = "- " + " ".join(f"word{i}" for i in range(15))
|
||||
violations = checker.check_body(f"## Caveats (if any)\n\n{bullet}\n", ())
|
||||
assert len(violations) == 1
|
||||
assert "15 words" in violations[0].detail
|
||||
|
||||
|
||||
def test_wrapped_bullet_continuation_counts_into_its_bullet(checker):
|
||||
body = "## Caveats (if any)\n\n- short bullet that\n wraps onto an indented line\n"
|
||||
assert checker.check_body(body, ()) == ()
|
||||
|
||||
|
||||
def test_none_line_in_caveats_passes(checker):
|
||||
assert checker.check_body("## Caveats (if any)\n\nNone\n", ()) == ()
|
||||
|
||||
|
||||
def test_qa_runbook_without_e2e_changes_fails(checker):
|
||||
body = "## QA runbook\n\n- some manual step\n"
|
||||
violations = checker.check_body(body, ("litellm/main.py",))
|
||||
assert len(violations) == 1
|
||||
assert "delete this section" in violations[0].detail
|
||||
|
||||
|
||||
def test_qa_runbook_with_e2e_changes_passes(checker):
|
||||
body = "## QA runbook\n\n- some manual step\n"
|
||||
assert checker.check_body(body, ("tests/e2e/test_thing.py",)) == ()
|
||||
|
||||
|
||||
def test_leftover_placeholder_fails(checker):
|
||||
violations = checker.check_body("## TLDR\n\nProblem this solves:\n\n- <blah>\n- ...\n", ())
|
||||
assert len(violations) == 2
|
||||
assert all("placeholder" in violation.detail for violation in violations)
|
||||
|
||||
|
||||
def test_html_comments_are_ignored(checker):
|
||||
body = "## Caveats (if any)\n\n<!-- Short bullet points, just like the TLDR: one line per bullet -->\n"
|
||||
assert checker.check_body(body, ()) == ()
|
||||
|
||||
|
||||
def test_final_attestation_checkbox_is_not_a_caveat(checker):
|
||||
body = (
|
||||
"## Caveats (if any)\n\n- a real caveat bullet\n\n### Final Attestation\n\n"
|
||||
"- [ ] The tests check the right things, including the edge cases, and regressions"
|
||||
" in the respective real-world customer use-cases are not possible after this PR\n"
|
||||
)
|
||||
assert checker.check_body(body, ()) == ()
|
||||
|
||||
|
||||
def test_empty_body_passes(checker):
|
||||
assert checker.check_body("", ()) == ()
|
||||
|
||||
|
||||
def test_multiline_html_comment_spanning_section_is_stripped(checker):
|
||||
body = "## QA runbook\n\n<!-- Only needed when your PR edits tests/e2e; delete this section otherwise\n\nExample:\n\n- step one\n-->\n"
|
||||
violations = checker.check_body(body, ("litellm/main.py",))
|
||||
assert len(violations) == 1
|
||||
assert violations[0].section == "QA runbook"
|
||||
Loading…
Add table
Reference in a new issue