From d0c8ba0659f1cc4cec16e637ccc07a42606338b5 Mon Sep 17 00:00:00 2001 From: taliesin-ai Date: Fri, 3 Jul 2026 23:44:13 +1000 Subject: [PATCH] refactor: address CodeRabbit review feedback - fold the PR-metric thresholds into Band.PRMinCredit so the band list and thresholds cannot drift apart and panic - gate the animated ring's hover-only bloom/fast layers on Band.Hover, matching the static branch - throttle the per-login lookupUser fallbacks like the paginated fetches, so changelog-only and PR-only lookups cannot burst into GitHub's secondary rate limits - table-driven unit tests for changelogMentions and Credit() --- tools/sponsorkit/config.go | 23 +++--- tools/sponsorkit/contributors.go | 4 + tools/sponsorkit/contributors_test.go | 104 ++++++++++++++++++++++++ tools/sponsorkit/main.go | 2 +- tools/sponsorkit/render_contributors.go | 10 ++- 5 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 tools/sponsorkit/contributors_test.go diff --git a/tools/sponsorkit/config.go b/tools/sponsorkit/config.go index 6787b50d8a6..2435414f22d 100644 --- a/tools/sponsorkit/config.go +++ b/tools/sponsorkit/config.go @@ -109,6 +109,10 @@ type Band struct { // for this band. Contributors land in the first band whose minimum // they meet. MinCredit int + // PRMinCredit replaces MinCredit when ranking by merged PRs + // (-metric prs): one merged PR is a much rarer unit than one commit, + // so the cut-off sits far lower. + PRMinCredit int // Avatar is the squircle's edge length in CSS pixels. Avatar float64 // Box is the horizontal space reserved per contributor. @@ -131,50 +135,45 @@ type Band struct { Hover bool } -// prBandMins replaces the bands' MinCredit thresholds when ranking by -// merged PRs (-metric prs): one merged PR is a much rarer unit than one -// commit, so the cut-offs sit far lower. -var prBandMins = []int{300, 100, 40, 15, 6, 2, 0} - // bands is ordered from most to least prolific, with MinCredit thresholds // tuned for the commit metric. The last entry is the catch-all for // first-time and drive-by contributors. var bands = []Band{ { - MinCredit: 1000, + MinCredit: 1000, PRMinCredit: 300, Avatar: 116, Box: 190, RowGap: 26, ShowName: true, NameSize: 15, MaxName: 22, Ring: "animated", RingWidth: 4.5, RingGradient: "ringPartner", Hover: true, }, { - MinCredit: 150, + MinCredit: 150, PRMinCredit: 100, Avatar: 94, Box: 152, RowGap: 24, ShowName: true, NameSize: 13.5, MaxName: 18, Ring: "animated", RingWidth: 4, RingGradient: "ringChampion", Hover: true, }, { - MinCredit: 60, + MinCredit: 60, PRMinCredit: 40, Avatar: 74, Box: 116, RowGap: 22, ShowName: true, NameSize: 12, MaxName: 15, Ring: "static", RingWidth: 3, RingGradient: "ringGold", Hover: true, }, { - MinCredit: 20, + MinCredit: 20, PRMinCredit: 15, Avatar: 58, Box: 74, RowGap: 16, Ring: "static", RingWidth: 2, RingGradient: "ringSilver", Hover: true, }, { - MinCredit: 8, + MinCredit: 8, PRMinCredit: 6, Avatar: 46, Box: 59, RowGap: 13, Ring: "subtle", RingWidth: 1.5, }, { - MinCredit: 3, + MinCredit: 3, PRMinCredit: 2, Avatar: 37, Box: 48, RowGap: 12, Ring: "subtle", RingWidth: 1.25, }, { - MinCredit: 0, + MinCredit: 0, PRMinCredit: 0, Avatar: 30, Box: 40, RowGap: 11, Ring: "subtle", RingWidth: 1, }, diff --git a/tools/sponsorkit/contributors.go b/tools/sponsorkit/contributors.go index ab846f671be..d68092f3d4a 100644 --- a/tools/sponsorkit/contributors.go +++ b/tools/sponsorkit/contributors.go @@ -109,6 +109,9 @@ func FetchContributors(client *http.Client, token, repo string, changelogs []str } // Authored merged PRs but absent from the commit history (e.g. // unlinked commit email). Same validation as changelog mentions. + // The pause matches the paginated fetches: bursts of per-login + // lookups are what trip GitHub's secondary rate limits. + time.Sleep(100 * time.Millisecond) u, err := lookupUser(client, token, key) if err != nil || u.Type != "User" || isBot(u.Login, u.Type) { continue @@ -132,6 +135,7 @@ func FetchContributors(client *http.Client, token, repo string, changelogs []str // Credited in a changelog but absent from the commit history (e.g. // their PR was squashed under someone else's authorship). Look the // login up so typos and non-user tokens are dropped. + time.Sleep(100 * time.Millisecond) u, err := lookupUser(client, token, key) if err != nil { fmt.Printf(" skipping changelog mention @%s: %v\n", key, err) diff --git a/tools/sponsorkit/contributors_test.go b/tools/sponsorkit/contributors_test.go new file mode 100644 index 00000000000..50b48ec71eb --- /dev/null +++ b/tools/sponsorkit/contributors_test.go @@ -0,0 +1,104 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func writeChangelog(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "changelog.mdx") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return path +} + +func TestChangelogMentions(t *testing.T) { + tests := []struct { + name string + content string + want map[string]int + }{ + { + name: "bare mentions, case-folded and counted", + content: "- Fixed X by @Alice\n- Fixed Y by @alice and @bob", + want: map[string]int{"alice": 2, "bob": 1}, + }, + { + name: "profile link trusts the URL, not the (typo'd) link text", + content: "- Fixed X [@hkere](https://github.com/hkhere)", + want: map[string]int{"hkhere": 1}, + }, + { + name: "profile link without @ in the text is not a credit", + content: "- See [the repo](https://github.com/wailsapp) for details", + want: map[string]int{}, + }, + { + name: "npm scopes and display handles are not logins", + content: "- Bump @wailsio/runtime\n- Fixed by @ronaldinho_x86", + want: map[string]int{}, + }, + { + name: "emails and mid-word @ are not mentions", + content: "- Contact lea.anthony@gmail.com or user@@host", + want: map[string]int{}, + }, + { + name: "trailing hyphen is trimmed", + content: "- Fixed by @carol-.", + want: map[string]int{"carol": 1}, + }, + { + name: "bracketed mention without a URL still counts", + content: "- Fixed by [@superDingda] in #4236", + want: map[string]int{"superdingda": 1}, + }, + { + name: "pull-request links are not profile credits", + content: "- Fix Z by @dave in [PR](https://github.com/wailsapp/wails/pull/1)", + want: map[string]int{"dave": 1}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := changelogMentions([]string{writeChangelog(t, tt.content)}) + if len(got) != len(tt.want) { + t.Fatalf("got %v, want %v", got, tt.want) + } + for login, n := range tt.want { + if got[login] != n { + t.Errorf("mentions[%q] = %d, want %d (full: %v)", login, got[login], n, got) + } + } + }) + } +} + +func TestCredit(t *testing.T) { + defer func(m string) { creditMetric = m }(creditMetric) + + tests := []struct { + name string + metric string + c Contributor + want int + }{ + {"commits metric takes max of commits and mentions", "commits", Contributor{Commits: 10, Mentions: 3}, 10}, + {"mentions win when larger", "commits", Contributor{Commits: 2, Mentions: 5}, 5}, + {"prs metric uses merged PRs", "prs", Contributor{Commits: 231, PRs: 26}, 26}, + {"prs metric still lets mentions win", "prs", Contributor{PRs: 25, Mentions: 90}, 90}, + {"commit-only contributors floored into the tail", "prs", Contributor{Commits: 50}, 7}, + {"small commit-only counts kept as-is", "prs", Contributor{Commits: 2}, 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + creditMetric = tt.metric + if got := tt.c.Credit(); got != tt.want { + t.Errorf("Credit() = %d, want %d", got, tt.want) + } + }) + } +} diff --git a/tools/sponsorkit/main.go b/tools/sponsorkit/main.go index 88a3458a9f5..46ed97e0422 100644 --- a/tools/sponsorkit/main.go +++ b/tools/sponsorkit/main.go @@ -56,7 +56,7 @@ func main() { case "prs": creditMetric = "prs" for i := range bands { - bands[i].MinCredit = prBandMins[i] + bands[i].MinCredit = bands[i].PRMinCredit } default: fmt.Fprintf(os.Stderr, "error: unknown -metric %q\n", *metric) diff --git a/tools/sponsorkit/render_contributors.go b/tools/sponsorkit/render_contributors.go index 56c4b867d48..ca2d4175c6a 100644 --- a/tools/sponsorkit/render_contributors.go +++ b/tools/sponsorkit/render_contributors.go @@ -223,10 +223,12 @@ func (r *renderer) contributor(c Contributor, bi int, cx, cy float64) { r.body.WriteString(fmt.Sprintf(`%s stroke="#FFFFFF" stroke-opacity="0.65" stroke-width="%s" stroke-linecap="round" stroke-dasharray="%s %s">`, ring, num(b.RingWidth*0.6), num(perim*0.12), num(perim*0.88), num(-perim))) // Hover-only bloom and fast counter-marching sweep. - r.body.WriteString(fmt.Sprintf(`%s class="bloom" stroke="url(#%s)" stroke-width="%s" filter="url(#soften)"/>`, - ring, b.RingGradient, num(b.RingWidth+7))) - r.body.WriteString(fmt.Sprintf(`%s class="fast" stroke="#FFFFFF" stroke-width="%s" stroke-linecap="round" stroke-dasharray="%s %s">`, - ring, num(b.RingWidth*0.7), num(perim*0.07), num(perim*0.93), num(perim))) + if b.Hover { + r.body.WriteString(fmt.Sprintf(`%s class="bloom" stroke="url(#%s)" stroke-width="%s" filter="url(#soften)"/>`, + ring, b.RingGradient, num(b.RingWidth+7))) + r.body.WriteString(fmt.Sprintf(`%s class="fast" stroke="#FFFFFF" stroke-width="%s" stroke-linecap="round" stroke-dasharray="%s %s">`, + ring, num(b.RingWidth*0.7), num(perim*0.07), num(perim*0.93), num(perim))) + } case "static": r.body.WriteString(fmt.Sprintf(`%s stroke="url(#%s)" stroke-width="%s"/>`, ring, b.RingGradient, num(b.RingWidth)))