fix(website): merge email-keyed leaderboard entries into matching github-keyed contributors - #2720
fix(website): merge email-keyed leaderboard entries into matching github-keyed contributors#2720Mohith26 wants to merge 3 commits into
Conversation
…hub-keyed contributors When some of a contributor's commits fail GitHub login resolution (e.g. the author email was not yet linked to their GitHub account at generation time), resolveIdentity() falls back to an email:<address> key and the leaderboard shows the same human as two ranked entries in the same range. Add a deterministic reconciliation pass after commit collection: collectContributorStats() now records the normalized author emails and names seen for each entry, and mergeEmailKeyedContributors() folds any leftover email:* entry into a github:* entry linked by that existing evidence (shared author email, shared author name, or author name equal to the GitHub login), summing commits, widening the date range, and keeping the github key. Ambiguous evidence shared by multiple github entries is never used, and no GitHub API calls are added. Fixes vllm-project#2699 Signed-off-by: Mohith Gajjela <109003762+Mohith26@users.noreply.github.com>
Exercises mergeEmailKeyedContributors() with the exact theohsiung split from issue vllm-project#2699 (github:theohsiung 34 commits + email:theobear870924@gmail.com 5 commits -> one 39-commit entry) and the Wilson Wu shared-name case, plus shared-email linking, case-insensitive matching, ambiguous-evidence and no-evidence non-merges, bot exclusion, and input-map immutability. Runs via the existing npm test target (node --test scripts/lib/*.test.mjs). Signed-off-by: Mohith Gajjela <109003762+Mohith26@users.noreply.github.com>
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
assigned
AayushSaini101,
drivebyer,
FAUST-BENCHOU,
ramkrishs,
shraderdm and
WUKUNTAI-0211
👥 vLLM Semantic Team NotificationThe following members have been identified for the changed files in this PR and have been automatically assigned when their GitHub accounts are assignable in this repository: 📁
|
drivebyer
left a comment
drivebyer
left a comment
There was a problem hiding this comment.
Nice find — the mechanism is real. A few notes from digging in:
- Both cases in #2699 have already self-healed (the API resolves those gmails today). The one live split left is
aby / chenzw0521@gmail.com, whose account was deleted — its PRs showuser: ghost, so both the commit API and the PR fallback come back empty. Your pass does fix that one. - Concern: matching on git author name can merge different people — two contributors both committing as
rootcollapse into one entry.AMBIGUOUSonly guards github↔github collisions. - Suggestion: keep just the name-equals-login path (that's what fixes both cases) and drop the name/email matches. Much smaller, and a lot harder to get wrong.
Per review: git author names are user-controlled and collide (two authors committing as 'root'), and the AMBIGUOUS guard only protects github-to-github collisions, so drop the email/name evidence matching and keep only the name-equals-login path. That path alone fixes the one live split (deleted account resolving to ghost); the original vllm-project#2699 splits have self-healed upstream. Tests updated to pin the deliberate non-merges. Signed-off-by: Mohith Gajjela <109003762+Mohith26@users.noreply.github.com>
|
Good points, thanks - done in 6df47db: dropped the email/name evidence matching and kept only the name-equals-login path, updated the tests to pin the deliberate non-merges (shared display name or shared email evidence alone no longer merge). All 8 tests green. |
|
Thanks for the quick turnaround! One correction to my earlier note, after re-checking the data: the pass as-is doesn't catch the one remaining live split — the email entry's author name is Suggestion: repurpose this PR as
Same outcome, much less machinery. |
7 participants
Fixes #2699
What
The contributor leaderboard splits one human into two ranked entries when some commits resolve to a GitHub login and others only to an email (e.g. theohsiung ranked twice in the same range). Root cause:
resolveIdentity()has no reconciliation betweengithub:*andemail:*keys.This adds a deterministic post-collection pass that folds leftover email-keyed entries into a matching github-keyed entry when linked by commit-author evidence the script already collects (shared author email, shared author name, or name equal to the login), summing commits and widening the date range. Ambiguous evidence shared by multiple github-keyed entries is never used, bots are excluded, and no new GitHub API calls are made.
The generated data file is not touched here; it regenerates on the next scheduled run.
Tests
New
node --testsuite inscripts/lib/(the repo's existing convention), including the exact theohsiung split from the issue (34 + 5 must merge into one 39-commit entry), non-merge cases, and immutability. 4 of 8 tests fail against the old pass-through behavior; 13/13 pass with the fix.npm run lintclean. Both commits are DCO signed-off.Credit to @theohsiung for the report in #2699.