fix(review): sum per-PR review-effort minutes with fallback in public-stats (#2070)

[RealDiligent]

Summary

Closes #2070

Completes the public-stats ROI fix: minutesSaved now sums each distinct published PR's reviewEffortMinutes, using MINUTES_SAVED_PER_PR only as a per-PR fallback when metadata is missing.

Root cause

#1955 persisted per-PR estimates, but getPublicStats still computed reviewed * AVG(minutes). SQLite's AVG skips NULLs, so mixed ledgers (some PRs with stored effort, some without) under-reported time saved — e.g. PR #10 with 4 min + PR #11 with no estimate reported 8 instead of 24.

Fix

• Change effort SQL from AVG(minutes) to SUM(COALESCE(minutes, MINUTES_SAVED_PER_PR)) at the per-PR grain (after deduping republish events)
• Use the sum directly for minutesSaved; preserve reviewed === 0 -> 0

Impact

Public ROI counter is credible during mixed rollout; fully-migrated ledgers behave the same as before when every PR has an estimate.

Test plan

• npm run typecheck
• npx vitest run test/unit/public-stats.test.ts (19/19)
• New D1 regression: one PR with reviewEffortMinutes + one without -> sum uses fallback per missing PR

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[loopover-orb]

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

_{Review updated: 2026-07-07 18:53:26 UTC}

2 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI pending · blocked

🛑 Suggested Action - Reject/Close

• AI reviewers agree on a likely critical defect: src/review/public-stats.ts ~line 322-327: once any allowlisted repo has published PRs, `minutesSaved` equals the own-ledger-only `minutesSavedTotal`, but `reviewed` (`reviewedOf(totals)`) includes Orb-fleet merged/closed counts folded in earlier (`totals.merged += orb.merged
• totals.closed += orb.closed`) — since the file's header states the own-ledger side is a frozen snapshot post-cutover while Orb keeps growing live ('the Orb aggregate only captures merged/closed, not reversals or a trailing-7-day split'), the public minutesSaved counter will stop scaling with reviewed once Orb activity grows, unlike the prior `reviewed * avgReviewEffortMinutes` formula
• this interaction is untested. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This PR correctly fixes the AVG-skips-NULL under-reporting bug from #1955 by switching the per-PR effort aggregate from AVG(minutes) to SUM(COALESCE(minutes, MINUTES_SAVED_PER_PR)) over the same repo/number-deduped subquery, and the new D1 regression test (one PR with a stored estimate, one without) directly proves the fix for the stated mixed-ledger scenario. However, the new SUM-based minutesSaved is sourced entirely from the own-ledger effort query, while `reviewed` (used to compute the total) also includes the Orb fleet aggregate folded in via getOrbGlobalStats; since the file's own header documents the own-ledger side as a frozen post-cutover snapshot while the Orb side keeps growing live, minutesSaved will plateau at the frozen own-ledger sum even as `reviewed` keeps climbing from Orb data — a real regression from the previous `reviewed * avgReviewEffortMinutes` formula, which at least scaled with the full reviewed count. No test in this diff exercises Orb rows together with a non-null own-ledger effort sum.

Blockers

• src/review/public-stats.ts ~line 322-327: once any allowlisted repo has published PRs, `minutesSaved` equals the own-ledger-only `minutesSavedTotal`, but `reviewed` (`reviewedOf(totals)`) includes Orb-fleet merged/closed counts folded in earlier (`totals.merged += orb.merged; totals.closed += orb.closed`) — since the file's header states the own-ledger side is a frozen snapshot post-cutover while Orb keeps growing live ('the Orb aggregate only captures merged/closed, not reversals or a trailing-7-day split'), the public minutesSaved counter will stop scaling with reviewed once Orb activity grows, unlike the prior `reviewed * avgReviewEffortMinutes` formula; this interaction is untested.

Nits — 4 non-blocking

• test/unit/public-stats.test.ts: the test titled "averages a real reviewEffortMinutes value out of metadata_json via json_extract (real D1)" still says "averages" in its title even though it now asserts summing behavior (sum(4,96)=100, not avg=50) — rename to match the new semantics this PR introduces.
• src/review/public-stats.ts: the local name `minutesSavedTotal` reads oddly next to `MINUTES_SAVED_PER_PR` (the per-PR fallback constant) — consider `effortSum` or similar so the per-PR-vs-total distinction is clearer at the call site.
• Add a D1 regression test that combines an own-ledger published PR with reviewEffortMinutes AND Orb-sourced rows (or add a comment explicitly scoping minutesSaved to own-ledger-only, decoupled from the Orb-inclusive `reviewed` total), so the Orb/own-ledger interaction is either verified or explicitly documented as a known tradeoff rather than silently diverging from the pre-PR growth behavior.
• If the intent is for the public ROI counter to keep growing with live Orb activity (as it did before this PR), consider scaling minutesSaved by `reviewed` using the own-ledger per-PR average the way the old formula did, rather than using a flat frozen SUM.

Why this is blocked

• src/review/public-stats.ts ~line 322-327: once any allowlisted repo has published PRs, `minutesSaved` equals the own-ledger-only `minutesSavedTotal`, but `reviewed` (`reviewedOf(totals)`) includes Orb-fleet merged/closed counts folded in earlier (`totals.merged += orb.merged; totals.closed += orb.closed`) — since the file's header states the own-ledger side is a frozen snapshot post-cutover while Orb keeps growing live ('the Orb aggregate only captures merged/closed, not reversals or a trailing-7-day split'), the public minutesSaved counter will stop scaling with reviewed once Orb activity grows, unlike the prior `reviewed * avgReviewEffortMinutes` formula; this interaction is untested.

Signal	Result	Evidence
Code review	❌ 1 blocker	1 reviewer
Linked issue	✅ Linked	#2070
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 266 registered-repo PR(s), 81 merged, 0 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	RealDiligent; Gittensor profile; 266 PR(s), 0 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: RealDiligent
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: JavaScript, Ruby, Svelte, TypeScript, Cuda, Markdown
• Official Gittensor activity: 266 PR(s), 0 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Keep the PR focused and include validation evidence before maintainer review.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review

[loopover-orb]

Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: src/review/public-stats.ts ~line 322-327: once any allowlisted repo has published PRs, `minutesSaved` equals the own-ledger-only `minutesSavedTotal`, but `reviewed` (`reviewedOf(totals)`) includes Orb-fleet merged/closed counts folded in earlier (`totals.merged += orb.merged; totals.closed += orb.closed`) — since the file's header states the own-ledger side is a frozen snapshot post-cutover while Orb keeps growing live ('the Orb aggregate only captures merged/closed, not reversals or a trailing-7-day split'), the public minutesSaved counter will stop scaling with reviewed once Orb activity grows, unlike the prior `reviewed * avgReviewEffortMinutes` formula; this interaction is untested.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.59%. Comparing base (0d1af9a) to head (9ee2a7d).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4058   +/-   ##
=======================================
  Coverage   93.59%   93.59%           
=======================================
  Files         379      379           
  Lines       35584    35585    +1     
  Branches    13050    13050           
=======================================
+ Hits        33304    33305    +1     
  Misses       1618     1618           
  Partials      662      662           

Files with missing lines	Coverage Δ
src/review/public-stats.ts	96.55% <100.00%> (+0.06%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.