You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
Mirror this precedent exactly: #8372, which fixed the identical bug shape for four sibling section
builders (buildCalibrationRecapSection, buildGateOutcomesRecapSection, buildPerRepoRecapSection, buildDriftRecapSection) — "shipped fully implemented + unit-tested but never composed into the delivered
digest." src/services/maintainer-recap-top-contributors.ts is the fifth sibling in that same family and it
fell through #8372's sweep untouched.
src/services/maintainer-recap-top-contributors.ts exports buildTopContributorsRecapSection (line 42),
described in its own header comment as: "Maintainer-recap TOP-CONTRIBUTORS section (#2244, content slice of
the #1963 recap digest)." It is fully implemented (dedup/rank/cap/public-safe-gate logic complete) and fully
unit-tested (test/unit/maintainer-recap-top-contributors.test.ts).
grep -rn "buildTopContributorsRecapSection" src/ (excluding test/) shows it is imported and called nowhere — confirmed the same way #8372 confirmed its four siblings were dead code. formatMaintainerRecap
(src/services/maintainer-recap.ts, the function that renders a RecapReport into the Discord/Slack digest
body) now calls all four of #8372's siblings (see the #8372 comment block at src/services/maintainer-recap.ts
lines 17-22 and their unconditional call sites around lines 177-217) but never calls buildTopContributorsRecapSection, and the emitted digest never contains a "## Top contributors" heading.
Unlike #8372's four siblings, this section needs data RecapReport does not currently carry: a per-login
tally of merged PRs across the scan window. That data source already exists and is already used for an
almost-identical per-login tally elsewhere in this codebase — mirror buildMaintainerQualityDashboard's contributorTotals pattern
(src/services/maintainer-quality-dashboard.ts lines 93, 142-153): it iterates a repo's pull requests,
tallies a Map<string, {...}> keyed on pr.authorLogin ?? "unknown", then sorts/caps at line 156-160 — the
exact same shape buildTopContributorsRecapSection already expects as its TopContributor[] input.
The raw per-repo merged-PR data to tally already exists via listRecentMergedPullRequests(env, fullName): Promise<RecentMergedPullRequestRecord[]>
(src/db/repositories.ts line 5090), whose records carry authorLogin and mergedAt
(src/types.ts line ~2081-2091) — no new D1 query or schema change is needed.
runMaintainerRecapJob (src/review/maintainer-recap-wire.ts, around lines 170-215) already loops repoNames once per digest run and loads two aggregators per repo (loadGatePrecisionReport, buildRepoOutcomeCalibration) into MaintainerRecapRepoInput[]. This is the natural place to also call listRecentMergedPullRequests per repo (same loop, same try/catch-per-repo fail-safe pattern already there)
and fold the results into a window-filtered (mergedAt within windowDays), login-tallied contributor list.
Requirements
Add a contributors: TopContributor[] (or equivalently-shaped) field to MaintainerRecapInputs / RecapReport (src/types.ts) carrying each login's merged-PR count for the scan window, computed by
tallying listRecentMergedPullRequests results per repo (filtered to mergedAt within the recap windowDays), mirroring buildMaintainerQualityDashboard's contributorTotals Map-tally pattern
(src/services/maintainer-quality-dashboard.ts lines 93, 142-153) — group by authorLogin ?? "unknown",
sum counts across every scan repo (a contributor active in multiple repos gets one combined total, not one
row per repo).
buildMaintainerRecap (src/services/maintainer-recap.ts) must fold this contributor data into the RecapReport it returns.
runMaintainerRecapJob (src/review/maintainer-recap-wire.ts) must gather the per-repo merged-PR data
(via listRecentMergedPullRequests) inside its existing per-repo loop, with the same per-repo
try/catch-and-skip fail-safe already used for the other two aggregators in that loop — a D1 hiccup on one
repo must not blank the whole digest, exactly like every other aggregator load in that function.
formatMaintainerRecap (src/services/maintainer-recap.ts) must call buildTopContributorsRecapSection
and render its ## Top contributors section into the digest, unconditionally (not behind an options flag) — same treatment fix(notifications): wire the maintainer-recap's calibration/gate-outcomes/per-repo section builders into the actual digest output #8372 gave calibrationSection/gateOutcomesSection, since the input
data is always present on every RecapReport once Requirement 1 lands. Route the section's title/lines
through the existing redactRecapLine/recapSectionLines helpers exactly like the other unconditional
sections do.
An empty/zero-contributor window must render the section's existing empty-state fallback line, not omit
the heading — mirror the recapSectionLines(..., "_No … for this window._") convention already used for
every other section in formatMaintainerRecap.
No change to what data is publicly emitted beyond what buildTopContributorsRecapSection itself already
redacts (it already gates every line through isPublicSafeText — do not weaken or duplicate that gate).
Deliverables
RecapReport/MaintainerRecapInputs (src/types.ts, src/services/maintainer-recap.ts) carry a
per-login merged-PR contributor tally for the window.
runMaintainerRecapJob (src/review/maintainer-recap-wire.ts) populates that tally from listRecentMergedPullRequests, window-filtered by mergedAt, with the same per-repo fail-safe
try/catch pattern as its sibling aggregator loads in the same loop.
formatMaintainerRecap unconditionally renders buildTopContributorsRecapSection's output as a ## Top contributors section, using the existing redactRecapLine/recapSectionLines helpers.
A unit test in test/unit/maintainer-recap-format.test.ts (or maintainer-recap.test.ts) asserting a formatMaintainerRecap output now DOES contain ## Top contributors with the expected ranked
login/merged lines for a report with contributor data, and DOES contain the section's empty-state
fallback line when the window has zero contributors.
A unit test in test/unit/maintainer-recap-wire.test.ts (or equivalent) asserting runMaintainerRecapJob/buildMaintainerRecap correctly aggregates merged-PR counts by login across
multiple repos in the window, and that a per-repo listRecentMergedPullRequests failure is
swallowed/skipped rather than aborting the whole digest (mirroring the existing maintainer_recap_repo_error fail-safe test coverage for the other two aggregators).
All deliverables above are required in this one PR — there is no narrower intentionally-deferred scope.
Test Coverage Requirements
99%+ Codecov patch coverage (branch-counted) on every changed/added line in src/**, measured unsharded via npm run test:coverage. This is a bug fix (dead code that was supposed to be live output, same class as #8372/#6636) — include a regression test that fails against the pre-fix behavior (asserting ## Top contributors is present in the rendered digest) and passes after the fix.
Expected Outcome
The maintainer-recap digest delivered to Discord/Slack now includes a "Top contributors" section (ranked
merged-PR counts by login, public-safe, capped and empty-state-handled) for every run that has contributor
data — closing the same "built, tested, never shipped" gap #8372 already closed for this section's four
siblings.
src/services/maintainer-recap-top-contributors.ts — the already-implemented, already-tested, never-wired
section builder (buildTopContributorsRecapSection).
src/services/maintainer-recap.ts — buildMaintainerRecap / formatMaintainerRecap (the composition
points to extend).
src/review/maintainer-recap-wire.ts — runMaintainerRecapJob (the per-repo data-loading loop to extend).
src/services/maintainer-quality-dashboard.ts lines 93, 142-160 — the contributorTotals per-login
Map-tally-sort-cap pattern to mirror.
src/db/repositories.ts line 5090 (listRecentMergedPullRequests) and src/types.ts
(RecentMergedPullRequestRecord, authorLogin/mergedAt) — the existing data source, no new query needed.
test/unit/maintainer-recap-top-contributors.test.ts — existing coverage for the section builder itself
(unaffected by this change).
Context
Mirror this precedent exactly: #8372, which fixed the identical bug shape for four sibling section
builders (
buildCalibrationRecapSection,buildGateOutcomesRecapSection,buildPerRepoRecapSection,buildDriftRecapSection) — "shipped fully implemented + unit-tested but never composed into the delivereddigest."
src/services/maintainer-recap-top-contributors.tsis the fifth sibling in that same family and itfell through #8372's sweep untouched.
src/services/maintainer-recap-top-contributors.tsexportsbuildTopContributorsRecapSection(line 42),described in its own header comment as: "Maintainer-recap TOP-CONTRIBUTORS section (#2244, content slice of
the #1963 recap digest)." It is fully implemented (dedup/rank/cap/public-safe-gate logic complete) and fully
unit-tested (
test/unit/maintainer-recap-top-contributors.test.ts).grep -rn "buildTopContributorsRecapSection" src/(excludingtest/) shows it is imported and callednowhere — confirmed the same way #8372 confirmed its four siblings were dead code.
formatMaintainerRecap(
src/services/maintainer-recap.ts, the function that renders aRecapReportinto the Discord/Slack digestbody) now calls all four of #8372's siblings (see the
#8372comment block atsrc/services/maintainer-recap.tslines 17-22 and their unconditional call sites around lines 177-217) but never calls
buildTopContributorsRecapSection, and the emitted digest never contains a "## Top contributors" heading.Unlike #8372's four siblings, this section needs data
RecapReportdoes not currently carry: a per-logintally of merged PRs across the scan window. That data source already exists and is already used for an
almost-identical per-login tally elsewhere in this codebase — mirror
buildMaintainerQualityDashboard'scontributorTotalspattern(
src/services/maintainer-quality-dashboard.tslines 93, 142-153): it iterates a repo's pull requests,tallies a
Map<string, {...}>keyed onpr.authorLogin ?? "unknown", then sorts/caps at line 156-160 — theexact same shape
buildTopContributorsRecapSectionalready expects as itsTopContributor[]input.The raw per-repo merged-PR data to tally already exists via
listRecentMergedPullRequests(env, fullName): Promise<RecentMergedPullRequestRecord[]>(
src/db/repositories.tsline 5090), whose records carryauthorLoginandmergedAt(
src/types.tsline ~2081-2091) — no new D1 query or schema change is needed.runMaintainerRecapJob(src/review/maintainer-recap-wire.ts, around lines 170-215) already loopsrepoNamesonce per digest run and loads two aggregators per repo (loadGatePrecisionReport,buildRepoOutcomeCalibration) intoMaintainerRecapRepoInput[]. This is the natural place to also calllistRecentMergedPullRequestsper repo (same loop, same try/catch-per-repo fail-safe pattern already there)and fold the results into a window-filtered (
mergedAtwithinwindowDays), login-tallied contributor list.Requirements
contributors: TopContributor[](or equivalently-shaped) field toMaintainerRecapInputs/RecapReport(src/types.ts) carrying each login's merged-PR count for the scan window, computed bytallying
listRecentMergedPullRequestsresults per repo (filtered tomergedAtwithin the recapwindowDays), mirroringbuildMaintainerQualityDashboard'scontributorTotalsMap-tally pattern(
src/services/maintainer-quality-dashboard.tslines 93, 142-153) — group byauthorLogin ?? "unknown",sum counts across every scan repo (a contributor active in multiple repos gets one combined total, not one
row per repo).
buildMaintainerRecap(src/services/maintainer-recap.ts) must fold this contributor data into theRecapReportit returns.runMaintainerRecapJob(src/review/maintainer-recap-wire.ts) must gather the per-repo merged-PR data(via
listRecentMergedPullRequests) inside its existing per-repo loop, with the same per-repotry/catch-and-skip fail-safe already used for the other two aggregators in that loop — a D1 hiccup on one
repo must not blank the whole digest, exactly like every other aggregator load in that function.
formatMaintainerRecap(src/services/maintainer-recap.ts) must callbuildTopContributorsRecapSectionand render its
## Top contributorssection into the digest, unconditionally (not behind anoptionsflag) — same treatment fix(notifications): wire the maintainer-recap's calibration/gate-outcomes/per-repo section builders into the actual digest output #8372 gavecalibrationSection/gateOutcomesSection, since the inputdata is always present on every
RecapReportonce Requirement 1 lands. Route the section'stitle/linesthrough the existing
redactRecapLine/recapSectionLineshelpers exactly like the other unconditionalsections do.
the heading — mirror the
recapSectionLines(..., "_No … for this window._")convention already used forevery other section in
formatMaintainerRecap.buildTopContributorsRecapSectionitself alreadyredacts (it already gates every line through
isPublicSafeText— do not weaken or duplicate that gate).Deliverables
RecapReport/MaintainerRecapInputs(src/types.ts,src/services/maintainer-recap.ts) carry aper-login merged-PR contributor tally for the window.
runMaintainerRecapJob(src/review/maintainer-recap-wire.ts) populates that tally fromlistRecentMergedPullRequests, window-filtered bymergedAt, with the same per-repo fail-safetry/catch pattern as its sibling aggregator loads in the same loop.
formatMaintainerRecapunconditionally rendersbuildTopContributorsRecapSection's output as a## Top contributorssection, using the existingredactRecapLine/recapSectionLineshelpers.test/unit/maintainer-recap-format.test.ts(ormaintainer-recap.test.ts) asserting aformatMaintainerRecapoutput now DOES contain## Top contributorswith the expected rankedlogin/merged lines for a report with contributor data, and DOES contain the section's empty-state
fallback line when the window has zero contributors.
test/unit/maintainer-recap-wire.test.ts(or equivalent) assertingrunMaintainerRecapJob/buildMaintainerRecapcorrectly aggregates merged-PR counts by login acrossmultiple repos in the window, and that a per-repo
listRecentMergedPullRequestsfailure isswallowed/skipped rather than aborting the whole digest (mirroring the existing
maintainer_recap_repo_errorfail-safe test coverage for the other two aggregators).All deliverables above are required in this one PR — there is no narrower intentionally-deferred scope.
Test Coverage Requirements
99%+ Codecov patch coverage (branch-counted) on every changed/added line in
src/**, measured unsharded vianpm run test:coverage. This is a bug fix (dead code that was supposed to be live output, same class as#8372/#6636) — include a regression test that fails against the pre-fix behavior (asserting
## Top contributorsis present in the rendered digest) and passes after the fix.Expected Outcome
The maintainer-recap digest delivered to Discord/Slack now includes a "Top contributors" section (ranked
merged-PR counts by login, public-safe, capped and empty-state-handled) for every run that has contributor
data — closing the same "built, tested, never shipped" gap #8372 already closed for this section's four
siblings.
Links & Resources
buildCalibrationRecapSection/buildGateOutcomesRecapSection/buildPerRepoRecapSection/buildDriftRecapSection.src/services/maintainer-recap-top-contributors.ts— the already-implemented, already-tested, never-wiredsection builder (
buildTopContributorsRecapSection).src/services/maintainer-recap.ts—buildMaintainerRecap/formatMaintainerRecap(the compositionpoints to extend).
src/review/maintainer-recap-wire.ts—runMaintainerRecapJob(the per-repo data-loading loop to extend).src/services/maintainer-quality-dashboard.tslines 93, 142-160 — thecontributorTotalsper-loginMap-tally-sort-cap pattern to mirror.
src/db/repositories.tsline 5090 (listRecentMergedPullRequests) andsrc/types.ts(
RecentMergedPullRequestRecord,authorLogin/mergedAt) — the existing data source, no new query needed.test/unit/maintainer-recap-top-contributors.test.ts— existing coverage for the section builder itself(unaffected by this change).