From f88eda40769146da41c16e2dcbe42f3996f6ea78 Mon Sep 17 00:00:00 2001 From: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Date: Sun, 21 Jun 2026 08:13:59 -0500 Subject: [PATCH 1/2] feat(auto-rebase-health): measure post-restriction fan-out reduction (#739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 instrumentation for epic #736. The Phase 1 report (#737) estimated fan-out as runs × ALL open non-Dependabot PRs, which does not reflect the review-ready eligibility gate that landed in petry-projects/.github#468 (issue #465) — so it could not show the reduction the gate buys. Add a "Post-restriction fan-out" section that mirrors the reusable's review-ready predicate against the current open PRs: - count eligible PRs (non-draft AND (current APPROVED review OR the auto-rebase:ready label)), reading actual review states (current-approval wins) exactly as the reusable's lib/eligibility.sh does - report the eligible-PR multiplier, the restricted re-run estimate, and the behind→eligible reduction, with an explicit ≥50% success-metric verdict This is the cleaner before/after signal that feeds the Merge Queue go/no-go decision record (#739). New pure helpers (fmt_reduction, pr_has_current_approval, count_eligible) are unit-tested; main() does the per-PR read I/O (no CI re-runs, no LLM cost, still ≤1 scheduled run/day). render_report stays backward-compatible — the new section renders only when an eligible count is supplied. shellcheck clean at the repo's --severity=warning gate; 28/28 bats pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019LUUSUQHqwLWZ583SAs41K --- scripts/auto_rebase_health.sh | 131 +++++++++++++++++++++++++++++++--- tests/auto_rebase_health.bats | 108 ++++++++++++++++++++++++++++ 2 files changed, 231 insertions(+), 8 deletions(-) diff --git a/scripts/auto_rebase_health.sh b/scripts/auto_rebase_health.sh index 7c60e90f7..eecd490bf 100644 --- a/scripts/auto_rebase_health.sh +++ b/scripts/auto_rebase_health.sh @@ -21,6 +21,15 @@ # logged in this repo (the update-branch calls live in the central reusable), # so the re-run volume is an ESTIMATE: runs × current open non-Dependabot PRs. # +# 3. Post-restriction fan-out — since petry-projects/.github#468 (issue #465) the +# reusable only update-branches *review-ready* PRs (non-draft AND (current +# APPROVED review OR the `auto-rebase:ready` label)). To show the reduction the +# report mirrors the same predicate against the current open PRs and reports the +# eligible-PR multiplier, the restricted re-run estimate, and the reduction vs. +# the unrestricted (all-behind-PRs) multiplier. This is the cleaner before/after +# signal feeding the Merge Queue go/no-go record (#739). Like (2) it is a +# snapshot ESTIMATE — eligibility is read now, not per historical run. +# # Layout (mirrors scripts/token_report.sh): # * The count_*/summarize_*/fmt_*/render_report functions are PURE — they take # JSON / scalars and write to stdout. Unit-tested in tests/auto_rebase_health.bats. @@ -31,6 +40,9 @@ # GH_PAT_FALLBACK — optional fallback PAT if GH_TOKEN lacks run-telemetry access # AGENT_REPO — repo to scan (default: petry-projects/.github-private) # LOOKBACK_DAYS — days of history to consider (default: 7) +# READY_LABEL — label that opts a non-draft PR into auto-rebase without an +# approval; must match the reusable's `ready_label` input +# (default: auto-rebase:ready) # AUTO_REBASE_HEALTH_OUT — optional path; report is written there in addition to stdout # GITHUB_STEP_SUMMARY — written by the Actions runner when present @@ -38,6 +50,7 @@ set -euo pipefail WORKFLOW_REPO="${AGENT_REPO:-petry-projects/.github-private}" LOOKBACK_DAYS="${LOOKBACK_DAYS:-7}" +READY_LABEL="${READY_LABEL:-auto-rebase:ready}" AUTO_REBASE_WORKFLOW="auto-rebase.yml" # Markers (kept in one place so a rename in the dev-lead scripts is a one-line fix). @@ -110,11 +123,58 @@ fmt_rate() { echo "$(( num * 100 / denom ))%" } -# render_report [today] +# fmt_reduction +# Integer percentage DECREASE going from to (e.g. 7→3 = "57%"). +# Zero/negative guard renders "n/a" (no PRs to reduce). Mirrors the +# behind→eligible multiplier reduction that the eligibility gate buys. +fmt_reduction() { + local from="${1:-0}" to="${2:-0}" + if [ "$from" -le 0 ]; then + echo "n/a" + return 0 + fi + echo "$(( (from - to) * 100 / from ))%" +} + +# pr_has_current_approval +# Returns 0 if a PR currently has at least one APPROVED review, else 1. Mirrors +# petry-projects/.github .github/scripts/auto-rebase/lib/eligibility.sh exactly: +# the reviewer's most recent decision review wins (a later CHANGES_REQUESTED or +# DISMISSED cancels an earlier APPROVED; COMMENTED/PENDING do not change a stance). +# We read real review states rather than reviewDecision, which is null on repos +# without required reviews (issue #465 implementer note). +pr_has_current_approval() { + local json="${1:-}" result + [ -n "$json" ] || json='[]' + result=$(printf '%s' "$json" | jq -r ' + reduce (.[] | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED")) as $r ({}; .[$r.user.login] = $r.state) + | any(. == "APPROVED")') + [ "$result" = "true" ] +} + +# count_eligible +# Counts review-ready PRs in a metadata array. Each element is +# {"draft":bool,"approved":bool,"labels":[{"name":...}]}; a PR is eligible when it +# is non-draft AND (approved OR carries ). Same predicate as the +# reusable's `review-ready` mode. Absent/empty JSON → 0. +count_eligible() { + local json="${1:-}" label="${2:-}" + [ -n "$json" ] || json='[]' + printf '%s' "$json" | jq --arg L "$label" ' + [ .[] + | select(((.draft // false) | not) + and ((.approved // false) or ([.labels[]?.name] | any(. == $L)))) ] + | length' +} + +# render_report [today] [eligible_prs] [ready_label] # Writes the full Markdown report to stdout. Pure: no network. +# When is supplied (non-empty), the post-restriction section is +# rendered; omit it to render the legacy two-section report. render_report() { local comments_json="${1:-[]}" runs_json="${2:-[]}" local lookback="${3:-7}" behind="${4:-0}" today="${5:-}" + local eligible="${6:-}" ready_label="${7:-auto-rebase:ready}" [ -n "$today" ] || today="$(date -u +%Y-%m-%d)" local sentinels responses applied @@ -149,6 +209,30 @@ render_report() { "$runs_per_day" "$rerun_per_day" printf '> Fan-out is an **estimate** — per-run behind-PR counts are not logged in this repo, ' printf 'so re-runs = runs × current open non-Dependabot PR count.\n' + + # Post-restriction section — only when an eligible-PR count is supplied. + [ -n "$eligible" ] || return 0 + + local restricted reduction restricted_per_day verdict + restricted="$(estimate_fanout "$total" "$eligible")" + reduction="$(fmt_reduction "$behind" "$eligible")" + restricted_per_day="$(awk -v t="$restricted" -v d="$lookback" 'BEGIN { printf "%.1f", (d > 0 ? t / d : 0) }')" + # ≥50% multiplier reduction is the epic #736 success metric. + if [ "$behind" -gt 0 ] && [ "$(( (behind - eligible) * 100 / behind ))" -ge 50 ]; then + verdict="✅ met" + else + verdict="❌ not met" + fi + + printf '\n## Post-restriction fan-out (review-ready eligibility)\n\n' + printf -- '- **Eligible PRs** (non-draft AND (current `APPROVED` review OR `%s` label)): %s of %s open non-Dependabot PRs\n' \ + "$ready_label" "$eligible" "$behind" + printf -- '- **Estimated branch-update CI re-runs (restricted)**: ~%s (~%s/day)\n' \ + "$restricted" "$restricted_per_day" + printf -- '- **Fan-out reduction** (behind→eligible multiplier): %s\n' "$reduction" + printf -- '- **≥50%% reduction success metric** (epic #736): %s\n\n' "$verdict" + printf '> Reduction is a point-in-time snapshot: eligibility is read now, not per ' + printf 'historical run, so the multiplier (not the absolute run count) is the like-for-like signal.\n' } # --------------------------------------------------------------------------- @@ -196,15 +280,46 @@ main() { --paginate --jq '.workflow_runs | map({conclusion, created_at})' 2>/dev/null \ | jq -s 'add // []' 2>/dev/null || echo '[]')" - # 3. Behind-PR multiplier — open non-Dependabot PRs (proxy for branches the - # fan-out updates). Best-effort; defaults to 0 so the report still renders. - local behind - behind="$(gh pr list --repo "$WORKFLOW_REPO" --state open --limit 200 --json author \ - --jq '[.[] | select((.author?.login // "") | test("dependabot"; "i") | not)] | length' \ - 2>/dev/null || echo 0)" + # 3. Behind-PR multiplier — open non-Dependabot PR numbers (proxy for branches + # the fan-out updates). Best-effort; empty list → behind=0 so it still renders. + local pr_numbers behind + pr_numbers="$(gh pr list --repo "$WORKFLOW_REPO" --state open --limit 200 --json number,author \ + --jq '.[] | select((.author?.login // "") | test("dependabot"; "i") | not) | .number' \ + 2>/dev/null || true)" + if [ -n "$pr_numbers" ]; then + behind="$(printf '%s\n' "$pr_numbers" | grep -c .)" + else + behind=0 + fi + + # 4. Eligible-PR multiplier — the review-ready subset the gate now updates. For + # each open non-Dependabot PR, read draft/labels (one pulls call) and the + # current approval state (reviews call), then apply the same predicate as the + # reusable. No CI re-runs and no LLM cost — a handful of read calls per daily + # run. Best-effort: any failure leaves eligible empty so the post-restriction + # section is simply omitted rather than reported wrongly. + local eligible="" prs_meta + if [ -n "$pr_numbers" ]; then + local recs="[]" n pr_json draft labels reviews_json approved rec + while IFS= read -r n; do + [ -n "$n" ] || continue + pr_json="$(gh api "repos/${WORKFLOW_REPO}/pulls/${n}" 2>/dev/null || echo '{}')" + draft="$(printf '%s' "$pr_json" | jq -r 'if .draft == true then "true" else "false" end')" + labels="$(printf '%s' "$pr_json" | jq -c '.labels // []')" + reviews_json="$(gh api --paginate "repos/${WORKFLOW_REPO}/pulls/${n}/reviews" 2>/dev/null \ + | jq -s 'add // []' 2>/dev/null || echo '[]')" + if pr_has_current_approval "$reviews_json"; then approved=true; else approved=false; fi + rec="$(jq -nc --argjson d "$draft" --argjson a "$approved" --argjson l "$labels" \ + '{draft:$d, approved:$a, labels:$l}')" + recs="$(printf '%s' "$recs" | jq -c --argjson r "$rec" '. + [$r]')" + done <<< "$pr_numbers" + prs_meta="$recs" + eligible="$(count_eligible "$prs_meta" "$READY_LABEL" 2>/dev/null || echo "")" + fi local report - report="$(render_report "$comments_json" "$runs_json" "$LOOKBACK_DAYS" "$behind" "$today")" + report="$(render_report "$comments_json" "$runs_json" "$LOOKBACK_DAYS" "$behind" "$today" \ + "$eligible" "$READY_LABEL")" printf '%s\n' "$report" if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then diff --git a/tests/auto_rebase_health.bats b/tests/auto_rebase_health.bats index 8a12a5a4a..6fc03f50a 100644 --- a/tests/auto_rebase_health.bats +++ b/tests/auto_rebase_health.bats @@ -30,6 +30,18 @@ setup() { {"conclusion":"success","created_at":"2026-06-11T00:00:00Z"}, {"conclusion":"success","created_at":"2026-06-11T06:00:00Z"} ]' + + # PR eligibility metadata: 4 open non-Dependabot PRs, 2 eligible. + # approved non-draft → eligible + # ready-labelled non-draft → eligible + # plain non-draft → not eligible + # approved but draft → not eligible (draft) + PRS_META_JSON='[ + {"draft":false,"approved":true,"labels":[]}, + {"draft":false,"approved":false,"labels":[{"name":"auto-rebase:ready"}]}, + {"draft":false,"approved":false,"labels":[{"name":"bug"}]}, + {"draft":true,"approved":true,"labels":[]} + ]' } # --------------------------------------------------------------------------- @@ -138,3 +150,99 @@ setup() { [ "$status" -eq 0 ] [[ "$output" == *"n/a"* ]] } + +# --------------------------------------------------------------------------- +# fmt_reduction — behind→eligible multiplier reduction +# --------------------------------------------------------------------------- + +@test "fmt_reduction: renders an integer percentage decrease" { + run fmt_reduction 8 2 + [ "$output" = "75%" ] +} + +@test "fmt_reduction: no reduction renders 0%" { + run fmt_reduction 5 5 + [ "$output" = "0%" ] +} + +@test "fmt_reduction: zero base renders n/a (no divide-by-zero)" { + run fmt_reduction 0 0 + [ "$output" = "n/a" ] +} + +# --------------------------------------------------------------------------- +# pr_has_current_approval — current review state wins +# --------------------------------------------------------------------------- + +@test "pr_has_current_approval: a lone APPROVED review counts as approved" { + run pr_has_current_approval '[{"user":{"login":"a"},"state":"APPROVED"}]' + [ "$status" -eq 0 ] +} + +@test "pr_has_current_approval: a later CHANGES_REQUESTED cancels an earlier APPROVED (same user)" { + run pr_has_current_approval '[{"user":{"login":"a"},"state":"APPROVED"},{"user":{"login":"a"},"state":"CHANGES_REQUESTED"}]' + [ "$status" -ne 0 ] +} + +@test "pr_has_current_approval: one user's APPROVED survives another's CHANGES_REQUESTED" { + run pr_has_current_approval '[{"user":{"login":"a"},"state":"APPROVED"},{"user":{"login":"b"},"state":"CHANGES_REQUESTED"}]' + [ "$status" -eq 0 ] +} + +@test "pr_has_current_approval: COMMENTED-only reviews are not an approval" { + run pr_has_current_approval '[{"user":{"login":"a"},"state":"COMMENTED"}]' + [ "$status" -ne 0 ] +} + +@test "pr_has_current_approval: empty/absent reviews are not an approval" { + run pr_has_current_approval "" + [ "$status" -ne 0 ] +} + +# --------------------------------------------------------------------------- +# count_eligible — non-draft AND (approved OR ready label) +# --------------------------------------------------------------------------- + +@test "count_eligible: counts non-draft approved-or-labelled PRs" { + run count_eligible "$PRS_META_JSON" "auto-rebase:ready" + [ "$status" -eq 0 ] + [ "$output" -eq 2 ] +} + +@test "count_eligible: a different ready label drops the label-only PR" { + run count_eligible "$PRS_META_JSON" "some-other-label" + [ "$output" -eq 1 ] +} + +@test "count_eligible: empty/absent JSON returns 0 (does not error)" { + run count_eligible "" "auto-rebase:ready" + [ "$status" -eq 0 ] + [ "$output" -eq 0 ] +} + +# --------------------------------------------------------------------------- +# render_report — post-restriction section (eligibility supplied) +# --------------------------------------------------------------------------- + +@test "render_report: post-restriction section appears only when eligible is supplied" { + run render_report "$COMMENTS_JSON" "$RUNS_JSON" 7 8 2026-06-15 + [[ "$output" != *"Post-restriction"* ]] + + run render_report "$COMMENTS_JSON" "$RUNS_JSON" 7 8 2026-06-15 2 auto-rebase:ready + [[ "$output" == *"Post-restriction fan-out"* ]] +} + +@test "render_report: post-restriction reports reduction and meets the ≥50% metric" { + # 8 behind → 2 eligible = 75% reduction (≥50% → met); 5 runs × 2 = 10 restricted re-runs + run render_report "$COMMENTS_JSON" "$RUNS_JSON" 7 8 2026-06-15 2 auto-rebase:ready + [ "$status" -eq 0 ] + [[ "$output" == *"75%"* ]] + [[ "$output" == *"met"* ]] + [[ "$output" == *"~10"* ]] +} + +@test "render_report: sub-50% reduction is reported as not met" { + # 4 behind → 3 eligible = 25% reduction (< 50% → not met) + run render_report "$COMMENTS_JSON" "$RUNS_JSON" 7 4 2026-06-15 3 auto-rebase:ready + [[ "$output" == *"not met"* ]] +} From 98fc98ec100b6993d9bb56ac688541dd712c2505 Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Sun, 21 Jun 2026 15:07:11 +0000 Subject: [PATCH 2/2] chore: apply manual instructions [skip ci-relay] --- scripts/auto_rebase_health.sh | 42 +++++++++++++++-------------------- tests/auto_rebase_health.bats | 10 +++++++++ 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/scripts/auto_rebase_health.sh b/scripts/auto_rebase_health.sh index eecd490bf..fd1b3e7a1 100644 --- a/scripts/auto_rebase_health.sh +++ b/scripts/auto_rebase_health.sh @@ -147,7 +147,7 @@ pr_has_current_approval() { local json="${1:-}" result [ -n "$json" ] || json='[]' result=$(printf '%s' "$json" | jq -r ' - reduce (.[] | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED")) as $r ({}; .[$r.user.login] = $r.state) + reduce (.[] | select((.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED") and .user?.login != null)) as $r ({}; .[$r.user.login] = $r.state) | any(. == "APPROVED")') [ "$result" = "true" ] } @@ -163,7 +163,7 @@ count_eligible() { printf '%s' "$json" | jq --arg L "$label" ' [ .[] | select(((.draft // false) | not) - and ((.approved // false) or ([.labels[]?.name] | any(. == $L)))) ] + and ((.approved // false) or any(.labels[]?; .name == $L))) ] | length' } @@ -280,39 +280,33 @@ main() { --paginate --jq '.workflow_runs | map({conclusion, created_at})' 2>/dev/null \ | jq -s 'add // []' 2>/dev/null || echo '[]')" - # 3. Behind-PR multiplier — open non-Dependabot PR numbers (proxy for branches - # the fan-out updates). Best-effort; empty list → behind=0 so it still renders. - local pr_numbers behind - pr_numbers="$(gh pr list --repo "$WORKFLOW_REPO" --state open --limit 200 --json number,author \ - --jq '.[] | select((.author?.login // "") | test("dependabot"; "i") | not) | .number' \ - 2>/dev/null || true)" - if [ -n "$pr_numbers" ]; then - behind="$(printf '%s\n' "$pr_numbers" | grep -c .)" - else - behind=0 - fi + # 3. Behind-PR multiplier — fetch isDraft and labels in the same list call so + # we need only one reviews call per PR (not two). Best-effort; empty list → + # behind=0 so it still renders. + local open_prs behind + open_prs="$(gh pr list --repo "$WORKFLOW_REPO" --state open --limit 200 \ + --json number,author,isDraft,labels \ + --jq '[.[] | select((.author?.login // "") | test("dependabot"; "i") | not)]' \ + 2>/dev/null || echo '[]')" + behind="$(printf '%s' "$open_prs" | jq 'length')" # 4. Eligible-PR multiplier — the review-ready subset the gate now updates. For - # each open non-Dependabot PR, read draft/labels (one pulls call) and the - # current approval state (reviews call), then apply the same predicate as the - # reusable. No CI re-runs and no LLM cost — a handful of read calls per daily - # run. Best-effort: any failure leaves eligible empty so the post-restriction + # each open non-Dependabot PR, read the current approval state (reviews call), + # then apply the same predicate as the reusable. No CI re-runs and no LLM cost. + # Best-effort: any failure leaves eligible empty so the post-restriction # section is simply omitted rather than reported wrongly. local eligible="" prs_meta - if [ -n "$pr_numbers" ]; then - local recs="[]" n pr_json draft labels reviews_json approved rec - while IFS= read -r n; do + if [ "$behind" -gt 0 ]; then + local recs="[]" n draft labels reviews_json approved rec + while IFS=$'\t' read -r n draft labels; do [ -n "$n" ] || continue - pr_json="$(gh api "repos/${WORKFLOW_REPO}/pulls/${n}" 2>/dev/null || echo '{}')" - draft="$(printf '%s' "$pr_json" | jq -r 'if .draft == true then "true" else "false" end')" - labels="$(printf '%s' "$pr_json" | jq -c '.labels // []')" reviews_json="$(gh api --paginate "repos/${WORKFLOW_REPO}/pulls/${n}/reviews" 2>/dev/null \ | jq -s 'add // []' 2>/dev/null || echo '[]')" if pr_has_current_approval "$reviews_json"; then approved=true; else approved=false; fi rec="$(jq -nc --argjson d "$draft" --argjson a "$approved" --argjson l "$labels" \ '{draft:$d, approved:$a, labels:$l}')" recs="$(printf '%s' "$recs" | jq -c --argjson r "$rec" '. + [$r]')" - done <<< "$pr_numbers" + done < <(printf '%s' "$open_prs" | jq -r '.[] | "\(.number)\t\(.isDraft)\t\(.labels | tostring)"') prs_meta="$recs" eligible="$(count_eligible "$prs_meta" "$READY_LABEL" 2>/dev/null || echo "")" fi diff --git a/tests/auto_rebase_health.bats b/tests/auto_rebase_health.bats index 6fc03f50a..f45d2e5fd 100644 --- a/tests/auto_rebase_health.bats +++ b/tests/auto_rebase_health.bats @@ -199,6 +199,16 @@ setup() { [ "$status" -ne 0 ] } +@test "pr_has_current_approval: null-user review is skipped without error" { + run pr_has_current_approval '[{"user":null,"state":"APPROVED"}]' + [ "$status" -ne 0 ] +} + +@test "pr_has_current_approval: null-user review does not block a valid approval" { + run pr_has_current_approval '[{"user":null,"state":"CHANGES_REQUESTED"},{"user":{"login":"a"},"state":"APPROVED"}]' + [ "$status" -eq 0 ] +} + # --------------------------------------------------------------------------- # count_eligible — non-draft AND (approved OR ready label) # ---------------------------------------------------------------------------