mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(triage): bugbot — drop dead gh_json and fix --optout-label append-with-default
- Removed the unused gh_json helper (bugbot low-severity dead code). - Replaced argparse `action="append", default=[...]` with default=None + DEFAULT_OPTOUT_LABELS fallback. The mutable-default + append combo silently APPENDS to the canonical defaults instead of replacing them, so --optout-label could not actually scope the opt-out list. - Added tests covering both the canonical default and the flag-replaces-defaults behavior. Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
This commit is contained in:
parent
09da8a7bf9
commit
c501f74a23
2 changed files with 92 additions and 10 deletions
24
.github/scripts/close_low_quality_prs.py
vendored
24
.github/scripts/close_low_quality_prs.py
vendored
|
|
@ -35,7 +35,7 @@ import json
|
|||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from typing import Any, Iterable
|
||||
from typing import Iterable
|
||||
|
||||
# Greptile's GitHub App appears as `greptile-apps[bot]` in REST API comments
|
||||
# and `greptile-apps` in `gh pr view --json` output. Accept either form.
|
||||
|
|
@ -54,6 +54,12 @@ SCORE_PATTERN = re.compile(
|
|||
# exempt from auto-triage.
|
||||
INTERNAL_AUTHOR_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
|
||||
|
||||
# Default labels that exempt a PR from auto-close. Defined at module scope (not
|
||||
# as a mutable argparse default) so that `--optout-label foo` REPLACES the
|
||||
# defaults instead of appending to them — the argparse `action="append"` +
|
||||
# `default=[...]` combination silently mutates the shared default list.
|
||||
DEFAULT_OPTOUT_LABELS = ("do not close", "keep open", "wip")
|
||||
|
||||
|
||||
def gh(*args: str) -> str:
|
||||
"""Run a `gh` CLI command and return stdout. Raises on non-zero exit."""
|
||||
|
|
@ -66,11 +72,6 @@ def gh(*args: str) -> str:
|
|||
return result.stdout
|
||||
|
||||
|
||||
def gh_json(*args: str) -> Any:
|
||||
"""Run a `gh` CLI command that emits JSON and return the parsed value."""
|
||||
return json.loads(gh(*args))
|
||||
|
||||
|
||||
def fetch_open_prs(repo: str | None) -> list[dict]:
|
||||
"""Fetch all open PRs (number, createdAt, isDraft, labels, author)."""
|
||||
repo_args = ["--repo", repo] if repo else []
|
||||
|
|
@ -297,10 +298,13 @@ def main() -> int:
|
|||
parser.add_argument(
|
||||
"--optout-label",
|
||||
action="append",
|
||||
default=["do not close", "keep open", "wip"],
|
||||
default=None,
|
||||
help=(
|
||||
"Label(s) that exempt a PR from auto-close. "
|
||||
"Repeat to add more. Case-insensitive."
|
||||
"Label(s) that exempt a PR from auto-close. Repeat to add more. "
|
||||
"Case-insensitive. When omitted, defaults to "
|
||||
f"{list(DEFAULT_OPTOUT_LABELS)!r}; passing this flag REPLACES the "
|
||||
"defaults (argparse `append` with a mutable default would append "
|
||||
"instead, which we explicitly avoid)."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
|
|
@ -334,7 +338,7 @@ def main() -> int:
|
|||
print(f"Found {len(prs)} open PRs.\n")
|
||||
|
||||
now = dt.datetime.now(dt.timezone.utc)
|
||||
optout_labels = set(args.optout_label)
|
||||
optout_labels = set(args.optout_label or DEFAULT_OPTOUT_LABELS)
|
||||
|
||||
closed = 0
|
||||
summary = {
|
||||
|
|
|
|||
|
|
@ -292,6 +292,84 @@ class TestEvaluatePr:
|
|||
assert score is None
|
||||
|
||||
|
||||
class TestMainOptoutLabelDefault:
|
||||
"""`--optout-label` must REPLACE the canonical defaults, not append."""
|
||||
|
||||
def _patch_no_op(self, closer_module, monkeypatch):
|
||||
monkeypatch.setattr(closer_module, "fetch_open_prs", lambda repo: [])
|
||||
# `optout_labels` is captured indirectly via evaluate_pr; sniff the
|
||||
# set passed in by stubbing evaluate_pr.
|
||||
captured: dict = {}
|
||||
|
||||
def fake_evaluate(pr, now, min_age_days, min_score, repo, optout_labels):
|
||||
captured["optout_labels"] = set(optout_labels)
|
||||
return ("skip-draft", None, None)
|
||||
|
||||
monkeypatch.setattr(closer_module, "evaluate_pr", fake_evaluate)
|
||||
return captured
|
||||
|
||||
def test_should_use_canonical_defaults_when_flag_omitted(
|
||||
self, closer_module, monkeypatch
|
||||
):
|
||||
captured = self._patch_no_op(closer_module, monkeypatch)
|
||||
# No PRs -> capture won't fire; instead inject one synthetic PR via
|
||||
# fetch_open_prs so evaluate_pr is invoked at least once.
|
||||
monkeypatch.setattr(
|
||||
closer_module,
|
||||
"fetch_open_prs",
|
||||
lambda repo: [
|
||||
{
|
||||
"number": 1,
|
||||
"title": "p",
|
||||
"createdAt": "2026-05-10T00:00:00Z",
|
||||
"isDraft": True,
|
||||
"labels": [],
|
||||
"author": {"login": "x"},
|
||||
}
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(sys, "argv", ["close_low_quality_prs.py"])
|
||||
rc = closer_module.main()
|
||||
assert rc == 0
|
||||
assert captured["optout_labels"] == set(closer_module.DEFAULT_OPTOUT_LABELS)
|
||||
|
||||
def test_should_replace_defaults_when_flag_provided(
|
||||
self, closer_module, monkeypatch
|
||||
):
|
||||
captured = self._patch_no_op(closer_module, monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
closer_module,
|
||||
"fetch_open_prs",
|
||||
lambda repo: [
|
||||
{
|
||||
"number": 1,
|
||||
"title": "p",
|
||||
"createdAt": "2026-05-10T00:00:00Z",
|
||||
"isDraft": True,
|
||||
"labels": [],
|
||||
"author": {"login": "x"},
|
||||
}
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
sys,
|
||||
"argv",
|
||||
[
|
||||
"close_low_quality_prs.py",
|
||||
"--optout-label",
|
||||
"hold",
|
||||
"--optout-label",
|
||||
"needs-discussion",
|
||||
],
|
||||
)
|
||||
rc = closer_module.main()
|
||||
assert rc == 0
|
||||
# Crucially, none of the canonical defaults leak in.
|
||||
assert captured["optout_labels"] == {"hold", "needs-discussion"}
|
||||
for default in closer_module.DEFAULT_OPTOUT_LABELS:
|
||||
assert default not in captured["optout_labels"], default
|
||||
|
||||
|
||||
class TestHasOptoutLabel:
|
||||
def test_should_match_label_case_insensitively(self, closer_module):
|
||||
pr = {"labels": [{"name": "Do Not Close"}, {"name": "bug"}]}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue