Track integration review decisions (#3394)

This commit is contained in:
zhifu gao
2026-07-23 13:48:44 +08:00
committed by GitHub
parent 07320adb85
commit de9ee9e94b
2 changed files with 299 additions and 8 deletions
+69 -8
View File
@@ -34,6 +34,7 @@ OPS_GITHUB_TOKEN_PATH = Path("/cpfs_speech/user/zhifu.gzf/.config/funasr-ops/git
DEFAULT_PYPI_DOWNLOAD_CACHE = Path("/cpfs_speech/user/zhifu.gzf/.cache/funasr-ops/pypi-downloads-funasr.json")
DEFAULT_INTEGRATION_PRS = [
"huggingface/transformers#46180",
"huggingface/optimum-intel#1874",
"yuekaizhang/Fun-ASR-vllm#21",
"ray-project/ray#64053",
"huggingface/speech-to-speech#319",
@@ -312,6 +313,7 @@ PASSIVE_INTEGRATION_ACTIONS = {
"wait for checks",
"wait for contributor branch update",
"wait for maintainer preliminary PR",
"wait for maintainer merge",
"wait for maintainer rerun",
"wait for maintainer review",
}
@@ -569,6 +571,48 @@ def classify_known_external_failure(spec: str, checks: Dict[str, Any]) -> Option
return None
def summarize_pull_request_reviews(repo: str, pr_number: int) -> Dict[str, Any]:
base_url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}/reviews?per_page=100"
reviews = []
page = 1
while True:
url = base_url if page == 1 else f"{base_url}&page={page}"
page_reviews = fetch_json(url, github_headers())
if not isinstance(page_reviews, list):
break
reviews.extend(page_reviews)
if len(page_reviews) < 100:
break
page += 1
latest_by_reviewer: Dict[str, str] = {}
for review in reviews:
reviewer = (review.get("user") or {}).get("login")
state = str(review.get("state") or "").upper()
if reviewer and state in {"APPROVED", "CHANGES_REQUESTED", "DISMISSED"}:
latest_by_reviewer[str(reviewer)] = state
approvals = sorted(
reviewer for reviewer, state in latest_by_reviewer.items() if state == "APPROVED"
)
changes_requested_by = sorted(
reviewer
for reviewer, state in latest_by_reviewer.items()
if state == "CHANGES_REQUESTED"
)
if changes_requested_by:
state = "changes_requested"
elif approvals:
state = "approved"
else:
state = "none"
return {
"state": state,
"approvals": approvals,
"changes_requested_by": changes_requested_by,
}
def recommend_integration_action(
pull_request: Dict[str, Any],
checks: Dict[str, Any],
@@ -576,6 +620,7 @@ def recommend_integration_action(
known_external_failure_action: Optional[str] = None,
known_review_gate_action: Optional[str] = None,
known_assisted_review_reason: Optional[str] = None,
review_state: Optional[str] = None,
) -> str:
if pull_request.get("state") != "open":
if pull_request.get("merged_at"):
@@ -631,6 +676,10 @@ def recommend_integration_action(
and mergeable_state in {"blocked", "clean", "unknown", "unstable", None}
):
return "wait for maintainer review"
if review_state == "changes_requested":
return "address review feedback"
if review_state == "approved":
return "wait for maintainer merge"
if check_state in {"success", "unknown"} and mergeable_state == "clean":
return "request review"
@@ -687,6 +736,22 @@ def collect_pull_request_metrics(spec: str, now: Optional[dt.datetime] = None) -
)
known_review_gate = KNOWN_REVIEW_GATES.get(spec) or {}
known_assisted_review = KNOWN_ASSISTED_REVIEW_REQUESTS.get(spec) or {}
recommendation_args = (
pull_request,
checks,
known_external_failure_reason,
known_external_failure_action,
known_review_gate.get("action"),
known_assisted_review.get("reason"),
)
review_summary = {"state": "not_checked", "approvals": [], "changes_requested_by": []}
next_action = recommend_integration_action(*recommendation_args)
if next_action in {"request review", "review gate"}:
review_summary = summarize_pull_request_reviews(repo, pr_number)
next_action = recommend_integration_action(
*recommendation_args,
review_state=review_summary["state"],
)
updated_at = pull_request.get("updated_at")
return {
"pr": f"{repo}#{pr_number}",
@@ -711,14 +776,10 @@ def collect_pull_request_metrics(spec: str, now: Optional[dt.datetime] = None) -
"known_external_failure_action": known_external_failure_action,
"known_review_gate_reason": known_review_gate.get("reason"),
"known_assisted_review_reason": known_assisted_review.get("reason"),
"next_action": recommend_integration_action(
pull_request,
checks,
known_external_failure_reason,
known_external_failure_action,
known_review_gate.get("action"),
known_assisted_review.get("reason"),
),
"review_state": review_summary["state"],
"approvals": review_summary["approvals"],
"changes_requested_by": review_summary["changes_requested_by"],
"next_action": next_action,
}
+230
View File
@@ -85,6 +85,7 @@ def test_default_integration_prs_include_high_visibility_external_queue():
module = load_growth_metrics_module()
expected_prs = {
"huggingface/optimum-intel#1874",
"TEN-framework/ten-framework#2191",
"Uberi/speech_recognition#903",
}
@@ -855,6 +856,8 @@ def test_collect_integration_metrics_recommends_review_for_clean_success_pr(monk
{"name": "validate", "status": "completed", "conclusion": "success"},
],
}
if url == "https://api.github.com/repos/example/clean-pr/pulls/42/reviews?per_page=100":
return []
raise AssertionError(f"unexpected URL: {url}")
monkeypatch.setattr(module, "fetch_json", fake_fetch_json)
@@ -864,6 +867,233 @@ def test_collect_integration_metrics_recommends_review_for_clean_success_pr(monk
assert metrics["integrations"][0]["next_action"] == "request review"
def test_collect_integration_metrics_waits_for_merge_after_maintainer_approval(monkeypatch):
module = load_growth_metrics_module()
def fake_fetch_json(url, headers=None):
if url == "https://api.github.com/repos/example/approved-pr/pulls/42":
return {
"number": 42,
"title": "Add FunASR",
"state": "open",
"draft": False,
"mergeable": True,
"mergeable_state": "clean",
"html_url": "https://github.com/example/approved-pr/pull/42",
"updated_at": "2026-07-23T04:16:43Z",
"head": {"sha": "1dafe5e", "ref": "add-funasr"},
"base": {"ref": "main"},
"user": {"login": "LauraGPT"},
}
if url == "https://api.github.com/repos/example/approved-pr/commits/1dafe5e/status":
return {"state": "success", "statuses": []}
if url == "https://api.github.com/repos/example/approved-pr/commits/1dafe5e/check-runs?per_page=100":
return {
"total_count": 1,
"check_runs": [
{"name": "validate", "status": "completed", "conclusion": "success"},
],
}
if url == "https://api.github.com/repos/example/approved-pr/pulls/42/reviews?per_page=100":
return [
{
"id": 123,
"state": "APPROVED",
"submitted_at": "2026-07-23T04:00:00Z",
"user": {"login": "maintainer"},
}
]
raise AssertionError(f"unexpected URL: {url}")
monkeypatch.setattr(module, "fetch_json", fake_fetch_json)
metrics = module.collect_integration_metrics(["example/approved-pr#42"])
integration = metrics["integrations"][0]
assert integration["review_state"] == "approved"
assert integration["approvals"] == ["maintainer"]
assert integration["next_action"] == "wait for maintainer merge"
def test_collect_integration_metrics_surfaces_requested_review_changes(monkeypatch):
module = load_growth_metrics_module()
def fake_fetch_json(url, headers=None):
if url == "https://api.github.com/repos/example/reviewed-pr/pulls/42":
return {
"number": 42,
"title": "Add FunASR",
"state": "open",
"draft": False,
"mergeable": True,
"mergeable_state": "clean",
"html_url": "https://github.com/example/reviewed-pr/pull/42",
"updated_at": "2026-07-23T04:16:43Z",
"head": {"sha": "824c3c5", "ref": "add-funasr"},
"base": {"ref": "main"},
"user": {"login": "LauraGPT"},
}
if url == "https://api.github.com/repos/example/reviewed-pr/commits/824c3c5/status":
return {"state": "success", "statuses": []}
if url == "https://api.github.com/repos/example/reviewed-pr/commits/824c3c5/check-runs?per_page=100":
return {
"total_count": 1,
"check_runs": [
{"name": "validate", "status": "completed", "conclusion": "success"},
],
}
if url == "https://api.github.com/repos/example/reviewed-pr/pulls/42/reviews?per_page=100":
return [
{
"id": 124,
"state": "CHANGES_REQUESTED",
"submitted_at": "2026-07-23T04:00:00Z",
"user": {"login": "maintainer"},
}
]
raise AssertionError(f"unexpected URL: {url}")
monkeypatch.setattr(module, "fetch_json", fake_fetch_json)
metrics = module.collect_integration_metrics(["example/reviewed-pr#42"])
integration = metrics["integrations"][0]
assert integration["review_state"] == "changes_requested"
assert integration["changes_requested_by"] == ["maintainer"]
assert integration["next_action"] == "address review feedback"
def test_review_summary_reads_later_pages_before_deciding(monkeypatch):
module = load_growth_metrics_module()
first_page = [
{
"id": 1,
"state": "CHANGES_REQUESTED",
"user": {"login": "maintainer"},
}
] + [
{
"id": review_id,
"state": "COMMENTED",
"user": {"login": f"reviewer-{review_id}"},
}
for review_id in range(2, 101)
]
def fake_fetch_json(url, headers=None):
if url == "https://api.github.com/repos/example/long-pr/pulls/42/reviews?per_page=100":
return first_page
if url == "https://api.github.com/repos/example/long-pr/pulls/42/reviews?per_page=100&page=2":
return [
{
"id": 101,
"state": "APPROVED",
"user": {"login": "maintainer"},
}
]
raise AssertionError(f"unexpected URL: {url}")
monkeypatch.setattr(module, "fetch_json", fake_fetch_json)
summary = module.summarize_pull_request_reviews("example/long-pr", 42)
assert summary["state"] == "approved"
assert summary["approvals"] == ["maintainer"]
assert summary["changes_requested_by"] == []
def test_review_summary_keeps_latest_decisive_state_per_reviewer(monkeypatch):
module = load_growth_metrics_module()
def fake_fetch_json(url, headers=None):
assert url == "https://api.github.com/repos/example/review-history/pulls/42/reviews?per_page=100"
return [
{"state": "CHANGES_REQUESTED", "user": {"login": "alice"}},
{"state": "COMMENTED", "user": {"login": "alice"}},
{"state": "APPROVED", "user": {"login": "alice"}},
{"state": "APPROVED", "user": {"login": "bob"}},
{"state": "COMMENTED", "user": {"login": "bob"}},
{"state": "APPROVED", "user": {"login": "carol"}},
{"state": "DISMISSED", "user": {"login": "carol"}},
]
monkeypatch.setattr(module, "fetch_json", fake_fetch_json)
summary = module.summarize_pull_request_reviews("example/review-history", 42)
assert summary["state"] == "approved"
assert summary["approvals"] == ["alice", "bob"]
assert summary["changes_requested_by"] == []
def test_blocked_approved_pr_waits_for_maintainer_merge(monkeypatch):
module = load_growth_metrics_module()
def fake_fetch_json(url, headers=None):
if url == "https://api.github.com/repos/example/blocked-pr/pulls/42":
return {
"number": 42,
"title": "Add FunASR",
"state": "open",
"draft": False,
"mergeable": True,
"mergeable_state": "blocked",
"html_url": "https://github.com/example/blocked-pr/pull/42",
"updated_at": "2026-07-23T04:16:43Z",
"head": {"sha": "abc1234", "ref": "add-funasr"},
"base": {"ref": "main"},
"user": {"login": "LauraGPT"},
}
if url == "https://api.github.com/repos/example/blocked-pr/commits/abc1234/status":
return {"state": "success", "statuses": []}
if url == "https://api.github.com/repos/example/blocked-pr/commits/abc1234/check-runs?per_page=100":
return {
"total_count": 1,
"check_runs": [
{"name": "test", "status": "completed", "conclusion": "success"}
],
}
if url == "https://api.github.com/repos/example/blocked-pr/pulls/42/reviews?per_page=100":
return [{"state": "APPROVED", "user": {"login": "maintainer"}}]
raise AssertionError(f"unexpected URL: {url}")
monkeypatch.setattr(module, "fetch_json", fake_fetch_json)
metrics = module.collect_integration_metrics(["example/blocked-pr#42"])
assert metrics["integrations"][0]["next_action"] == "wait for maintainer merge"
def test_wait_for_maintainer_merge_is_not_active_operator_work():
module = load_growth_metrics_module()
metrics = {
"collected_at_utc": "2026-07-23T06:00:00+00:00",
"integrations": [
{
"pr": "huggingface/optimum-intel#1874",
"html_url": "https://github.com/huggingface/optimum-intel/pull/1874",
"state": "open",
"mergeable": True,
"mergeable_state": "clean",
"repo_stars": 608,
"repo_forks": 147,
"updated_at": "2026-07-23T04:16:43Z",
"updated_age_days": 0,
"checks": {
"state": "success",
"failed_check_runs": [],
"pending_check_runs": [],
},
"next_action": "wait for maintainer merge",
}
],
}
output = module.format_integration_markdown(metrics)
assert "## Active operator queue" not in output
def test_known_assisted_review_requests_include_validated_discovery_prs():
module = load_growth_metrics_module()