diff --git a/platform/lib/queries/org-summary.ts b/platform/lib/queries/org-summary.ts index 9cfc879..c52e17b 100644 --- a/platform/lib/queries/org-summary.ts +++ b/platform/lib/queries/org-summary.ts @@ -1153,14 +1153,14 @@ export function computeHyperEngineers( { names: Set; github?: string; - repos: number; + repoIds: Set; hvWeeks: number; aiPct: number; aiCount: number; } >(); - for (const [, p] of payloads) { + for (const [repoId, p] of payloads) { if (!p.author_velocity?.authors) continue; const av = p.author_velocity; @@ -1168,13 +1168,21 @@ export function computeHyperEngineers( if (!isHyperEngineer(a)) continue; const nameLower = a.name.toLowerCase(); - const github = nameToGithub.get(nameLower); + // nameToGithub alone misses a real case: some people's local git + // config uses their GitHub handle itself as the commit author name + // (e.g. author name "lucastribioliclickbus" instead of "Lucas + // Tribioli") in repos where the push-time API resolution didn't tie + // that name's email to a login. userMap is *also* keyed by github + // username when known, so checking it here — not just at display + // time — catches that case during grouping instead of after. + const github = + nameToGithub.get(nameLower) ?? userMap.get(nameLower)?.github; const key = github ?? normalizeEmailIdentity(a.email) ?? nameLower; const existing = authors.get(key) ?? { names: new Set(), github: undefined, - repos: 0, + repoIds: new Set(), hvWeeks: 0, aiPct: 0, aiCount: 0, @@ -1184,7 +1192,11 @@ export function computeHyperEngineers( // userMap having a matching entry, since nameToGithub can know a // mapping userMap's own dedup pass didn't happen to retain. if (github) existing.github = github; - existing.repos++; + // A Set, not a counter: the same repo can list this person under two + // raw name/email variants (e.g. a noreply email in some commits and a + // corporate email in others) — once merged under one identity, that + // repo must still only count once. + existing.repoIds.add(repoId); existing.hvWeeks = Math.max(existing.hvWeeks, a.high_velocity_weeks); existing.aiPct += a.ai_commit_pct; existing.aiCount++; @@ -1201,12 +1213,22 @@ export function computeHyperEngineers( const userInfo = userMap.get(key) ?? [...a.names].map((n) => userMap.get(n.toLowerCase())).find(Boolean); + // Prefer a real name (has a space) over a bare username string — + // some repos record the commit author as the person's GitHub handle + // itself (e.g. "lucastribioliclickbus"), which sorts longest but + // reads worse than "Lucas Tribioli". const displayName = - userInfo?.name ?? [...a.names].sort((x, y) => y.length - x.length)[0]; + userInfo?.name ?? + [...a.names].sort((x, y) => { + const xHasSpace = x.includes(" "); + const yHasSpace = y.includes(" "); + if (xHasSpace !== yHasSpace) return xHasSpace ? -1 : 1; + return y.length - x.length; + })[0]; return { name: displayName, github: a.github ?? userInfo?.github, - repos: a.repos, + repos: a.repoIds.size, highVelocityWeeks: a.hvWeeks, aiCommitPct: a.aiCount > 0 ? a.aiPct / a.aiCount : 0, }; diff --git a/platform/tests/org-summary.test.ts b/platform/tests/org-summary.test.ts index 495933b..0524efa 100644 --- a/platform/tests/org-summary.test.ts +++ b/platform/tests/org-summary.test.ts @@ -467,6 +467,94 @@ describe("computeHyperEngineers — dedupes the same person across name variants expect(result).toHaveLength(2); }); + it("merges when a raw commit author name is literally the GitHub handle, resolved via userMap at grouping time (not just display time)", () => { + // Real case: some repos record the commit author as "Lucas Tribioli" + // (noreply email, resolves via nameToGithub for free); others record it + // as "lucastribioliclickbus" (his own GitHub handle used as the local + // git config name) with a corporate email that push-time API + // resolution failed to tie to a login. nameToGithub has no entry for + // that raw handle-as-name string, but userMap does (keyed by github + // username, populated from the repo that resolved successfully) — the + // grouping key computation must check userMap too, not just at display + // time, or these end up as two separate cards for the same person. + const payloads = new Map([ + [ + "repo-a", + payload({ + author_velocity: { + authors: [ + hyperAuthor({ + name: "Lucas Tribioli", + email: "999+lucastribioliclickbus@users.noreply.github.com", + }), + ], + }, + }), + ], + [ + "repo-b", + payload({ + author_velocity: { + authors: [ + hyperAuthor({ + name: "lucastribioliclickbus", + email: "lucas.tribioli@clickbus.com", + }), + ], + }, + }), + ], + ]); + const nameToGithub = new Map([["lucas tribioli", "lucastribioliclickbus"]]); + const userMap = new Map([ + [ + "lucastribioliclickbus", + { name: "Lucas Tribioli", github: "lucastribioliclickbus" }, + ], + ]); + + const result = computeHyperEngineers(payloads, userMap, nameToGithub); + + expect(result).toHaveLength(1); + expect(result[0].github).toBe("lucastribioliclickbus"); + expect(result[0].name).toBe("Lucas Tribioli"); + expect(result[0].repos).toBe(2); + }); + + it("counts a repo once even when the same merged identity appears under two name/email variants within that one repo", () => { + const payloads = new Map([ + [ + "repo-a", + payload({ + author_velocity: { + authors: [ + hyperAuthor({ + name: "Lucas Tribioli", + email: "999+lucastribioliclickbus@users.noreply.github.com", + }), + hyperAuthor({ + name: "lucastribioliclickbus", + email: "lucas.tribioli@clickbus.com", + }), + ], + }, + }), + ], + ]); + const nameToGithub = new Map([["lucas tribioli", "lucastribioliclickbus"]]); + const userMap = new Map([ + [ + "lucastribioliclickbus", + { name: "Lucas Tribioli", github: "lucastribioliclickbus" }, + ], + ]); + + const result = computeHyperEngineers(payloads, userMap, nameToGithub); + + expect(result).toHaveLength(1); + expect(result[0].repos).toBe(1); + }); + it("still includes an engineer with no resolved GitHub username, with github left undefined", () => { // computeHyperEngineers doesn't filter these out — the "identified" vs // "unidentified" split is a display concern, done in HyperEngineers.tsx.