From 5d9f28c48c9d3a9f7a6fde059e8176857f5b2b42 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:45:16 -0700 Subject: [PATCH] ci: gate PR bodies on the template's HTML comment rules --- .github/scripts/check_pr_body.py | 154 ++++++++++++++++++ .github/workflows/pr-body-template.yml | 47 ++++++ .../test_litellm/test_github_check_pr_body.py | 111 +++++++++++++ 3 files changed, 312 insertions(+) create mode 100644 .github/scripts/check_pr_body.py create mode 100644 .github/workflows/pr-body-template.yml create mode 100644 tests/test_litellm/test_github_check_pr_body.py diff --git a/.github/scripts/check_pr_body.py b/.github/scripts/check_pr_body.py new file mode 100644 index 00000000000..70881e69bcf --- /dev/null +++ b/.github/scripts/check_pr_body.py @@ -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 = ("",) +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.+?)\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()) diff --git a/.github/workflows/pr-body-template.yml b/.github/workflows/pr-body-template.yml new file mode 100644 index 00000000000..0a93cef386c --- /dev/null +++ b/.github/workflows/pr-body-template.yml @@ -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 diff --git a/tests/test_litellm/test_github_check_pr_body.py b/tests/test_litellm/test_github_check_pr_body.py new file mode 100644 index 00000000000..edf3587e42f --- /dev/null +++ b/tests/test_litellm/test_github_check_pr_body.py @@ -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"