fix(notifications): wire maintainer-recap's top-contributors section builder into the actual digest output - #9394
Conversation
…builder into the actual digest output Fixes JSONbored#9291
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-27 17:13:52 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9394 +/- ##
==========================================
- Coverage 75.46% 75.44% -0.02%
==========================================
Files 275 278 +3
Lines 58030 58168 +138
Branches 6199 6219 +20
==========================================
+ Hits 43790 43883 +93
- Misses 13970 14012 +42
- Partials 270 273 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Summary
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.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 plan
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.Fixes #9291