fix(ci): run the risk gate on pull_request and follow the one-pager rubric

The workflow now triggers on pull_request instead of pull_request_target, which the auto-approve one-pager and the 2026-06-18 CI exposure post-mortem both forbid, and the job skips fork PRs since the read-only token cannot label them. The floor now follows the rubric's author row (Devin-opened low, human-opened medium, fork high) through a new authors.low list, and adds a paths.medium list for agent instruction files, conftest, helm, and docker. Two guards compare both sides of a file through git show: model map rows that change or disappear stop being low, and a budget file that raises a limit stops being low. A skip or importorskip call added to an existing test file with no net new test scores high. The last 400 staging merges score 218 high, 179 medium, 3 low; the author row alone moves 37 human-opened docs and test PRs from low to medium
This commit is contained in:
mateo-berri 2026-09-07 17:57:01 -07:00
parent 6edc2c1611
commit 7064b2bf09
4 changed files with 291 additions and 20 deletions

View file

@ -35,6 +35,13 @@ paths:
- "**/package*.json"
- "**/Dockerfile*"
- "scripts/*gate*"
medium:
- "**/CLAUDE.md"
- "**/AGENTS.md"
- "**/GEMINI.md"
- "**/conftest.py"
- "helm/**"
- "docker/**"
low:
- "cookbook/**"
- "model_prices_and_context_window.json"
@ -65,3 +72,14 @@ tests:
- "**/*.test.*"
- "**/*.spec.*"
- "**/__tests__/**"
authors:
low:
- "devin-ai-integration[bot]"
guards:
additive_rows:
- "model_prices_and_context_window.json"
- "litellm/model_prices_and_context_window_backup.json"
lowered_limits:
- "*-budget.json"

View file

@ -5,23 +5,26 @@ import json
import re
import subprocess
import sys
from collections.abc import Sequence
from dataclasses import dataclass
from collections.abc import Callable, Mapping, Sequence
from dataclasses import dataclass, replace
from pathlib import Path
from types import MappingProxyType
from typing import Final, Literal, TypeAlias
import yaml
from pydantic import BaseModel, ConfigDict, Field
from pydantic import BaseModel, ConfigDict, Field, TypeAdapter, ValidationError
from typing_extensions import ReadOnly, TypedDict
Tier: TypeAlias = Literal["low", "medium", "high"]
ReadFile: TypeAlias = Callable[[str, str], str | None]
RewriteCheck: TypeAlias = Callable[[str | None, str | None], str | None]
TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(")
SKIP_RE: Final = re.compile(
r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b"
r"|\b(?:it|test|describe)\.(?:skip\(\s*[\"'`]|only\()|\bx(?:it|test|describe)\("
)
SKIP_CALL_RE: Final = re.compile(r"\bpytest\.(?:skip|importorskip)\(|\b(?:it|test|describe)\.fixme\(")
ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(")
GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)")
DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git "
@ -32,6 +35,7 @@ QUOTED_PATH_ESCAPES: Final = MappingProxyType(
{"a": "\a", "b": "\b", "f": "\f", "n": "\n", "r": "\r", "t": "\t", "v": "\v", '"': '"', "\\": "\\"}
)
MAX_LISTED_PATHS: Final = 5
JSON_OBJECT: Final = TypeAdapter(dict[str, object])
class SizeLimit(BaseModel):
@ -56,6 +60,7 @@ class ModuleRules(BaseModel):
class PathRules(BaseModel):
model_config = ConfigDict(extra="forbid", frozen=True)
high: tuple[str, ...]
medium: tuple[str, ...] = ()
low: tuple[str, ...]
@ -64,12 +69,25 @@ class TestRules(BaseModel):
files: tuple[str, ...]
class AuthorRules(BaseModel):
model_config = ConfigDict(extra="forbid", frozen=True)
low: tuple[str, ...]
class GuardRules(BaseModel):
model_config = ConfigDict(extra="forbid", frozen=True)
additive_rows: tuple[str, ...] = ()
lowered_limits: tuple[str, ...] = ()
class RiskConfig(BaseModel):
model_config = ConfigDict(extra="forbid", frozen=True)
paths: PathRules
modules: ModuleRules
size: SizeRules
tests: TestRules
authors: AuthorRules
guards: GuardRules = GuardRules()
@dataclass(frozen=True, slots=True)
@ -90,6 +108,7 @@ class FileChange:
added_lines: tuple[str, ...]
deleted_lines: tuple[str, ...]
previous_path: str | None = None
guarded_rewrite: str | None = None
@property
def paths(self) -> tuple[str, ...]:
@ -104,29 +123,45 @@ class FileChange:
class Rules:
config: RiskConfig
high_paths: PathMatcher
medium_paths: PathMatcher
low_paths: PathMatcher
test_files: PathMatcher
size_ignored: PathMatcher
additive_rows: PathMatcher
lowered_limits: PathMatcher
@staticmethod
def from_config(config: RiskConfig) -> Rules:
return Rules(
config=config,
high_paths=PathMatcher.from_globs(config.paths.high),
medium_paths=PathMatcher.from_globs(config.paths.medium),
low_paths=PathMatcher.from_globs(config.paths.low),
test_files=PathMatcher.from_globs(config.tests.files),
size_ignored=PathMatcher.from_globs(config.size.ignore),
additive_rows=PathMatcher.from_globs(config.guards.additive_rows),
lowered_limits=PathMatcher.from_globs(config.guards.lowered_limits),
)
def path_tier(self, path: str) -> Tier:
if self.high_paths.matches(path):
return "high"
if self.medium_paths.matches(path):
return "medium"
if self.test_files.matches(path) or self.low_paths.matches(path):
return "low"
return "medium"
def change_tier(self, change: FileChange) -> Tier:
return highest(tuple(self.path_tier(path) for path in change.paths))
rewrite_tier: Final[Tier] = "low" if change.guarded_rewrite is None else "medium"
return highest((rewrite_tier, *(self.path_tier(path) for path in change.paths)))
def rewrite_check(self, path: str) -> RewriteCheck | None:
if self.additive_rows.matches(path):
return rewritten_rows
if self.lowered_limits.matches(path):
return raised_limits
return None
def is_production(self, change: FileChange) -> bool:
return self.change_tier(change) != "low"
@ -263,7 +298,14 @@ def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor:
)
return Factor("paths", "high", f"always-human: {_listed(always_human)}")
case "medium":
return Factor("paths", "medium", f"{len(matching)} file(s) outside the docs, tests, and model map tiers")
rewritten: Final = tuple(
f"{change.guarded_rewrite} in `{change.path}`" for change in matching if change.guarded_rewrite
)
outside: Final = len(matching) - len(rewritten)
outside_note: Final = (
(f"{outside} file(s) outside the docs, tests, and model map tiers",) if outside else ()
)
return Factor("paths", "medium", "; ".join((*rewritten, *outside_note)))
case "low":
return Factor("paths", "low", "docs, tests, cookbook, or model map only")
@ -312,12 +354,17 @@ def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor:
test_changes: Final = tuple(change for change in changes if rules.test_files.matches(change.path))
net_tests: Final = _net(test_changes, TEST_DEF_RE)
net_skips: Final = _net(test_changes, SKIP_RE)
silenced: Final = sum(
max(_net((change,), SKIP_CALL_RE), 0) for change in test_changes if _net((change,), TEST_DEF_RE) <= 0
)
net_asserts: Final = _net(test_changes, ASSERT_RE)
production_changed: Final = any(rules.is_production(change) for change in changes)
if net_tests < 0:
return Factor("tests", "high", f"{-net_tests} test(s) removed")
if net_skips > 0:
return Factor("tests", "high", f"{net_skips} skip marker(s) added")
if silenced > 0:
return Factor("tests", "high", f"{silenced} skip call(s) added to existing tests")
if net_asserts < 0:
return Factor("tests", "high", f"{-net_asserts} assertion(s) removed")
if not production_changed:
@ -329,10 +376,12 @@ def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor:
return Factor("tests", "medium", "production code changed with no test touched")
def author_factor(author: str, from_fork: bool) -> Factor:
def author_factor(author: str, from_fork: bool, rules: Rules) -> Factor:
if from_fork:
return Factor("author", "high", f"`{author}` from a fork")
return Factor("author", "low", f"`{author}` on an internal branch")
if author in rules.config.authors.low:
return Factor("author", "low", f"`{author}` opened it on an internal branch")
return Factor("author", "medium", f"`{author}` opened it by hand on an internal branch")
def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict:
@ -341,11 +390,88 @@ def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules:
modules_factor(changes, rules),
size_factor(changes, rules),
tests_factor(changes, rules),
author_factor(author, from_fork),
author_factor(author, from_fork, rules),
)
return Verdict(highest(tuple(factor.tier for factor in factors)), factors)
def _json_object(text: str | None) -> Mapping[str, object] | None:
if text is None:
return None
try:
return MappingProxyType(JSON_OBJECT.validate_json(text))
except ValidationError:
return None
def _as_object(value: object) -> Mapping[str, object] | None:
try:
return MappingProxyType(JSON_OBJECT.validate_python(value))
except ValidationError:
return None
def rewritten_rows(base_text: str | None, head_text: str | None) -> str | None:
if base_text is None:
return None
base: Final = _json_object(base_text)
head: Final = _json_object(head_text)
if base is None or head is None:
return "not a JSON object on both sides"
rewritten: Final = sum(1 for key, value in base.items() if key not in head or head[key] != value)
return f"{rewritten} existing row(s) changed or removed" if rewritten else None
def _limits(value: object, prefix: str = "") -> tuple[tuple[str, float], ...]:
node: Final = _as_object(value)
if node is None:
return ()
limit: Final = node.get("limit")
own: Final = ((prefix, float(limit)),) if isinstance(limit, int | float) and not isinstance(limit, bool) else ()
nested: Final = tuple(
pair for key, child in node.items() if key != "limit" for pair in _limits(child, f"{prefix}/{key}")
)
return (*own, *nested)
def raised_limits(base_text: str | None, head_text: str | None) -> str | None:
if base_text is None:
return None
base: Final = _json_object(base_text)
head: Final = _json_object(head_text)
if base is None or head is None:
return "not a JSON object on both sides"
base_limits: Final = MappingProxyType(dict(_limits(base)))
raised: Final = sum(1 for key, limit in _limits(head) if key not in base_limits or limit > base_limits[key])
return f"{raised} limit(s) raised" if raised else None
def with_guards(
changes: Sequence[FileChange], rules: Rules, base: str, head: str, read_file: ReadFile
) -> tuple[FileChange, ...]:
return tuple(_guarded(change, rules, base, head, read_file) for change in changes)
def _guarded(change: FileChange, rules: Rules, base: str, head: str, read_file: ReadFile) -> FileChange:
check: Final = rules.rewrite_check(change.path)
if check is None:
return change
reason: Final = check(read_file(base, change.previous_path or change.path), read_file(head, change.path))
return change if reason is None else replace(change, guarded_rewrite=reason)
def git_show(repo: Path, rev: str, path: str) -> str | None:
completed: Final = subprocess.run(
("git", "show", f"{rev}:{path}"),
cwd=repo,
capture_output=True,
encoding="utf-8",
errors="replace",
check=False,
)
return completed.stdout if completed.returncode == 0 else None
def git_diff(repo: Path, base: str, head: str) -> str:
completed: Final = subprocess.run(
("git", "-c", "core.quotePath=false", "diff", "--find-renames", "--no-ext-diff", "-U0", base, head),
@ -384,7 +510,13 @@ class CliArgs(BaseModel):
def main(argv: Sequence[str] | None = None) -> int:
args: Final = CliArgs.model_validate(vars(build_parser().parse_args(argv)))
rules: Final = Rules.from_config(load_config(args.config))
changes: Final = parse_diff(git_diff(args.repo, args.base, args.head))
changes: Final = with_guards(
parse_diff(git_diff(args.repo, args.base, args.head)),
rules,
args.base,
args.head,
lambda rev, path: git_show(args.repo, rev, path),
)
verdict: Final = classify(changes, args.author, args.from_fork, rules)
if args.json_out is not None:
args.json_out.write_text(verdict.to_json(), encoding="utf-8")

View file

@ -1,7 +1,7 @@
name: risk-gate
on: # zizmor: ignore[dangerous-triggers] runs the base branch's copy of the gate only; the PR's diff is read as data and never executed
pull_request_target:
on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
branches:
- litellm_internal_staging
@ -16,6 +16,7 @@ concurrency:
jobs:
floor:
name: risk-gate floor (shadow)
if: github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest
timeout-minutes: 10
permissions:
@ -43,23 +44,17 @@ jobs:
with:
version: "0.10.9"
- name: Compute the floor tier with the base branch's copy of the gate
- name: Compute the floor tier
env:
MERGE_BASE: ${{ steps.revisions.outputs.merge_base }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
FROM_FORK: ${{ github.event.pull_request.head.repo.full_name != github.repository }}
run: |
fork_flag=()
if [ "$FROM_FORK" = "true" ]; then
fork_flag=(--from-fork)
fi
uv run --frozen python .github/scripts/risk_tier.py \
--config .github/risk-tiers.yml \
--base "$MERGE_BASE" \
--head "$HEAD_SHA" \
--author "$PR_AUTHOR" \
"${fork_flag[@]}" \
--json-out "${RUNNER_TEMP}/risk.json" \
| tee -a "$GITHUB_STEP_SUMMARY"

View file

@ -236,6 +236,7 @@ def test_unconditional_skip_marker_is_high(risk_tier, rules, marker):
' pytest.skip("needs a real key")',
'@unittest.skipIf(sys.platform == "win32", "posix only")',
'@unittest.skipUnless(HAS_REDIS, "needs redis")',
'redis = pytest.importorskip("redis")',
],
)
def test_conditional_skip_is_not_a_silenced_test(risk_tier, rules, guard):
@ -247,6 +248,29 @@ def test_conditional_skip_is_not_a_silenced_test(risk_tier, rules, guard):
assert factor.reason == "1 test(s) added"
@pytest.mark.parametrize(
("path", "line"),
[
("tests/test_litellm/test_a.py", ' pytest.skip("flaky since the refactor, see LIT-0000")'),
("tests/test_litellm/test_a.py", 'redis = pytest.importorskip("redis")'),
("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", " test.fixme(true, 'broken after the redesign');"),
],
)
def test_skip_call_added_to_an_existing_test_is_high_even_without_production_code(risk_tier, rules, path, line):
verdict = _verdict(risk_tier, rules, _file_diff(path, added=(line,)))
assert verdict.tier == "high"
assert _factor(verdict, "tests").reason == "1 skip call(s) added to existing tests"
def test_removing_a_skip_call_is_not_silencing(risk_tier, rules):
diff = _file_diff("tests/test_litellm/test_a.py", deleted=(' pytest.skip("flaky")',)) + _file_diff(
"litellm/llms/anthropic/chat/x.py", added=("x = 1",)
)
factor = _factor(_verdict(risk_tier, rules, diff), "tests")
assert factor.tier == "medium"
assert factor.reason == "tests edited, none added"
def test_weakened_assertions_are_high(risk_tier, rules):
diff = _file_diff(
"tests/test_litellm/test_a.py",
@ -311,7 +335,7 @@ def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expect
("author", "from_fork", "expected"),
[
(DEVIN, False, "low"),
("mateo-berri", False, "low"),
("mateo-berri", False, "medium"),
(DEVIN, True, "high"),
("jairandresdiazp", True, "high"),
],
@ -355,7 +379,17 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected):
("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", "low"),
("tests/e2e/x.py", "low"),
("litellm-proxy-extras/tests/test_x.py", "low"),
("helm/litellm-helm/tests/x.yaml", "low"),
("helm/litellm-helm/tests/x.yaml", "medium"),
("helm/litellm-helm/templates/tests/test-connection.yaml", "medium"),
("docker/tests/nonroot.yaml", "medium"),
("docker/entrypoint.sh", "medium"),
("CLAUDE.md", "medium"),
("AGENTS.md", "medium"),
("GEMINI.md", "medium"),
("litellm/proxy/_experimental/mcp_server/CLAUDE.md", "medium"),
("tests/conftest.py", "medium"),
("tests/test_litellm/conftest.py", "medium"),
("tests/e2e/junit_properties.py", "low"),
("ui/litellm-dashboard/tests/x.spec.ts", "low"),
("litellm-rust/crates/core/tests/x.rs", "low"),
("enterprise/litellm_enterprise/proxy/common_utils/x.py", "medium"),
@ -371,6 +405,73 @@ def test_path_tier_from_the_checked_in_config(rules, path, expected):
assert rules.path_tier(path) == expected
def test_human_opened_docs_change_is_medium_on_the_author_factor_only(risk_tier, rules):
verdict = _verdict(risk_tier, rules, _file_diff("README.md", added=("hello",)), author="mateo-berri")
assert verdict.tier == "medium"
assert {factor.name: factor.tier for factor in verdict.factors} == {
"paths": "low",
"modules": "low",
"size": "low",
"tests": "low",
"author": "medium",
}
MODEL_MAP = "model_prices_and_context_window.json"
BUDGET = "ruff-strict-budget.json"
def _guarded_verdict(risk_tier, rules, path: str, base: str | None, head: str | None):
contents = {("base", path): base, ("head", path): head}
diff = _file_diff(path, added=("changed",))
changes = risk_tier.with_guards(risk_tier.parse_diff(diff), rules, "base", "head", lambda rev, p: contents[(rev, p)])
return risk_tier.classify(changes, DEVIN, False, rules)
def test_additive_model_map_rows_stay_low(risk_tier, rules):
base = json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}})
head = json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}, "gpt-y": {"input_cost_per_token": 2e-6}})
verdict = _guarded_verdict(risk_tier, rules, MODEL_MAP, base, head)
assert verdict.tier == "low"
assert _factor(verdict, "paths").reason == "docs, tests, cookbook, or model map only"
@pytest.mark.parametrize(
"head",
[
{"gpt-x": {"input_cost_per_token": 3e-6}},
{"gpt-x-renamed": {"input_cost_per_token": 1e-6}},
{},
],
)
def test_changed_or_removed_model_map_rows_are_medium(risk_tier, rules, head):
base = json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}})
verdict = _guarded_verdict(risk_tier, rules, MODEL_MAP, base, json.dumps(head))
assert verdict.tier == "medium"
assert _factor(verdict, "paths").tier == "medium"
assert _factor(verdict, "paths").reason == f"1 existing row(s) changed or removed in `{MODEL_MAP}`"
def test_new_model_map_file_or_broken_json_is_never_low(risk_tier, rules):
assert _guarded_verdict(risk_tier, rules, MODEL_MAP, None, "{}").tier == "low"
assert _factor(_guarded_verdict(risk_tier, rules, MODEL_MAP, "{}", "not json"), "paths").tier == "medium"
assert _factor(_guarded_verdict(risk_tier, rules, MODEL_MAP, "{}", None), "paths").tier == "medium"
def test_lowered_budget_limits_stay_low_and_raised_ones_are_medium(risk_tier, rules):
base = json.dumps({"ANN001": {"limit": 10}, "B006": {"limit": 5}})
lowered = json.dumps({"ANN001": {"limit": 9}, "B006": {"limit": 5}})
raised = json.dumps({"ANN001": {"limit": 10}, "B006": {"limit": 6}})
new_rule = json.dumps({"ANN001": {"limit": 10}, "B006": {"limit": 5}, "B999": {"limit": 1}})
dropped_rule = json.dumps({"ANN001": {"limit": 10}})
assert _guarded_verdict(risk_tier, rules, BUDGET, base, lowered).tier == "low"
assert _guarded_verdict(risk_tier, rules, BUDGET, base, dropped_rule).tier == "low"
for head in (raised, new_rule):
verdict = _guarded_verdict(risk_tier, rules, BUDGET, base, head)
assert verdict.tier == "medium"
assert _factor(verdict, "paths").reason == f"1 limit(s) raised in `{BUDGET}`"
def test_single_star_does_not_cross_directories(risk_tier):
assert risk_tier.glob_to_regex("Dockerfile*").fullmatch("Dockerfile.database")
assert risk_tier.glob_to_regex("Dockerfile*").fullmatch("docker/Dockerfile.database") is None
@ -513,6 +614,31 @@ def test_main_end_to_end_against_a_git_repo(risk_tier, tmp_path, capsys):
assert payload["summary"] == printed
def test_main_reads_both_sides_of_the_model_map_from_git(risk_tier, tmp_path):
repo = tmp_path / "repo"
repo.mkdir()
_git(tmp_path, "init", "-q", "-b", "main", "repo")
model_map = repo / "model_prices_and_context_window.json"
model_map.write_text(json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}}, indent=1))
_git(repo, "add", ".")
_git(repo, "commit", "-q", "-m", "base")
base = _git(repo, "rev-parse", "HEAD")
model_map.write_text(json.dumps({"gpt-x": {"input_cost_per_token": 2e-6}}, indent=1))
_git(repo, "commit", "-q", "-am", "reprice")
head = _git(repo, "rev-parse", "HEAD")
json_out = tmp_path / "risk.json"
exit_code = risk_tier.main(
["--config", str(CONFIG_PATH), "--base", base, "--head", head, "--author", DEVIN, "--repo", str(repo), "--json-out", str(json_out)]
)
assert exit_code == 0
payload = json.loads(json_out.read_text())
assert payload["tier"] == "medium"
paths = next(factor for factor in payload["factors"] if factor["name"] == "paths")
assert paths["reason"] == "1 existing row(s) changed or removed in `model_prices_and_context_window.json`"
def test_git_quoted_filename_still_reaches_the_paths_factor(risk_tier, rules, tmp_path):
repo = tmp_path / "repo"
(repo / "litellm" / "proxy" / "auth").mkdir(parents=True)