fix(review): compute public-stats accuracyPct from own-ledger merged/closed only - #7474
Conversation
…closed only getPublicStats computed the global totals.accuracyPct from the Orb-fleet-folded totals.merged/closed (denominator) against the own-ledger-only totals.reversed (numerator). getOrbGlobalStats has no reversal concept, so the denominator grew with every newly registered install while the numerator stayed own-ledger-scoped, trending the published accuracy toward 100% independent of real reversal behavior. Snapshot the pre-fold own-ledger merged/closed (beside the existing ownLedgerReviewed snapshot) and compute the global accuracyPct from those, so numerator and denominator are drawn from the same population (option 1 of the issue). The fleet fold still inflates reviewed/handled/minutesSaved, which have no numerator/denominator pairing; accuracyPct's formula and byProject's per-project values are unchanged. Adds a regression test asserting a huge Orb-fleet fold no longer pulls accuracyPct toward 100. Closes JSONbored#7449
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7474 +/- ##
==========================================
- Coverage 91.36% 91.35% -0.02%
==========================================
Files 717 717
Lines 73016 73018 +2
Branches 21628 21628
==========================================
- Hits 66713 66704 -9
- Misses 5265 5272 +7
- Partials 1038 1042 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-20 11:46:34 UTC
Review summary Nits — 3 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. 🟩 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.
|
Summary
Fixes #7449.
getPublicStatscomputed the homepage's globaltotals.accuracyPctasaccuracyPct(totals.merged, totals.closed, totals.reversed), but those inputs are drawn from structurally different, independently-growing populations:totals.merged/totals.closedare folded with the whole registered Orb fleet's counts (viagetOrbGlobalStats, which has no reversal concept), whiletotals.reversedstays own-ledger-only. SinceaccuracyPctis1 - min(1, reversed / (merged + closed)), the denominator grows with every newly registered install while the numerator stays fixed to the own-ledger repos — so the published accuracy trended toward 100% as more self-hosted installations registered, independent of the fleet's actual reversal behavior.Fix (option 1 from the issue)
Snapshot the pre-fold own-ledger
merged/closed(right beside the existingownLedgerReviewedsnapshot) and compute the globalaccuracyPctfrom those, so its numerator and denominator come from the same population. The Orb-fleet fold still (correctly) inflates the rawreviewed/handled/minutesSavedtotals — those have no numerator/denominator pairing and are working as intended per the file's own comments.accuracyPct's formula/signature is untouched, and per-projectbyProject[].accuracyPctwas already same-scope (each computed from its own merged/closed/reversed) and is unchanged. A code comment at the call site (and at the snapshot) documents the choice, matching the file's house style of documenting non-obvious metric-composition decisions.Option 2 (fold a fleet-wide reversal count into
totals.reversed) was not chosen:getOrbGlobalStats(src/orb/outcomes.ts) reports only{ merged, closed, total }with no reversal tracking, so option 2 would require a schema/query extension for no additional accuracy over option 1.Tests
test/unit/public-stats.test.tsadds a regression test running the realgetPublicStatsend-to-end (own-ledger disposition + reversal reads and theorb_pr_outcomesaggregate all mocked): 100 own-ledger decided PRs with 10 real reversals (a true 90% accuracy) plus a huge 10,000-PR Orb fleet fold. It asserts the raw counts still fold (merged/closed/handled) butaccuracyPctis90, not the pre-fix fleet-inflated99, and that per-project accuracy stays90. All existinggetPublicStatsfield tests (reviewed/handled/minutesSaved/per-projectaccuracyPct, the Orb-fold tests) pass unchanged. Every changed line + branch is covered (verified via--coverage), socodecov/patchholds.Validation
git diff --checknpm run typecheck(roottsc --noEmit) greennpm run test:coverageon the affected suite: 100% of changed lines + branches covered; existingpublic-statsunit + integration-route tests pass unmodifiedsrc/review/public-stats.ts+ its unit test only. The returned stats object's fields/shape are unchanged (sameaccuracyPct: number | null), so no OpenAPI/schema/generated artifact needs regenerationSafety
site//CNAME/lovablechanges.Closes #7449