fix(stats): withhold reversal-grounded accuracy where a reversal cannot be recorded - #9718
Merged
Merged
Conversation
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 06:42:12 UTC
Review summary Nits — 7 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
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
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy · Diff highlights exactly what changed. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 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.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Owner
Author
|
Corrected the UI Evidence: the |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 70c8d94 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 06:24 AM |
Bundle ReportChanges will increase total bundle size by 1.43kB (0.02%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9718 +/- ##
==========================================
- Coverage 90.27% 89.46% -0.82%
==========================================
Files 904 904
Lines 113177 113195 +18
Branches 26840 26849 +9
==========================================
- Hits 102171 101265 -906
- Misses 9676 10842 +1166
+ Partials 1330 1088 -242
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This was referenced Jul 29, 2026
…ot be recorded The fairness page showed 100% accuracy for every repository and all eight weeks of the trend while the headline read 94.6%. Those 100%s are structural zeros, not measurements. `recordReversalSignals` has one call site, inside `handlePullRequestWebhookEvent`, reached only via the `github-webhook` job — a member of REVIEW_EXECUTION_JOB_TYPES, which a runtime that does not execute reviews acks-and-drops. So `reversed` is pinned at 0 while merged/closed keep growing, and `1 - 0/N` renders as a flawless score over a numerator that can never move. The repo already recorded the outcome: reversal-superseded.ts notes "verified in production: zero reversal events ever". `accuracyPct` now returns null unless a reversal is OBSERVABLE on this deployment — a recorded reversal proves it outright (and short-circuits the probe), otherwise there must be at least one terminal auto-action in the retained window for a reversal to attach to. This is the discipline the surface already applies elsewhere, extended to the case where the numerator's WRITER, not just its sample, is absent: a sparse rule's precision is null, and gamingFlagsCaught is null below three instances. Also fixes the weekly trend's denominator asymmetry: `merged`/`closed` folded in registered-Orb rows while `reversed` stayed own-ledger-only, so the denominator grew with every new install while the numerator did not — trending every week toward 100% independent of real reversal behavior. #7449 fixed exactly this for the lifetime figure and it was never carried across. Accuracy now divides the own-ledger pairing; the displayed volume still includes the fleet. UI: the withheld cells say why rather than leaving a bare em dash next to healthy volume, and the headline now discloses `basis` — #9168 computes "single_instance_self_report" precisely so one operator's self-reported number is not read as fleet corroboration, and the page dropped the field entirely, along with the sample size and confidence interval. The methodology note also described the reversal formula while the headline rendered decisionAccuracy; it now explains both and why they differ. Refs #9676
JSONbored
force-pushed
the
fix/fairness-structural-zeros
branch
from
July 29, 2026 06:21
3e198e7 to
70c8d94
Compare
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…r survives in #9718 gated accuracy on whether a reversal is observable at all, and fixed the weekly trend's Orb-folded denominator. It did not fix the third asymmetry in #9676: `merged`/`closed` come from `github_app.pr_public_surface_published`, the single retention-EXEMPT audit event type, so they are lifetime and never shrink -- while `reversal_*` rows prune with the rest of audit_events at 90 days. Pairing an immortal denominator with a 90-day numerator makes `1 - reversed/decided` drift toward 100% as the ledger ages, independent of real reversal behavior. Confirmed live after #9718 deployed: byProject still published 100% for all three repos on 2377/602/508 reviewed with 0 reversals, because the reversal signal IS observable on that deployment -- the gate passed and the ratio was still meaningless. The accuracy denominator is now a second, retention-windowed disposition query that shares BOTH of the numerator's bounds: own-ledger only, and inside audit_events' retention window. `reviewed`/`merged`/`closed` keep publishing lifetime volume, which is measured and correct. That display-vs-denominator split is the same shape #7449 established for the Orb fold and #9718 carried into the weekly trend; the #7449 snapshot it supersedes is removed rather than left dead, with its reasoning folded into the new comment. Boundary case, stated rather than hidden: a PR published before the window but reversed inside it contributes to the numerator and not the denominator. accuracyPct's existing clamp already handles that (it is the same shape as the reopened-auto-close case its comment describes), and the alternative -- an unbounded denominator -- is the bug being fixed. Refs #9676
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…r survives in (#9768) * fix(stats): bound the accuracy denominator to the window its numerator survives in #9718 gated accuracy on whether a reversal is observable at all, and fixed the weekly trend's Orb-folded denominator. It did not fix the third asymmetry in #9676: `merged`/`closed` come from `github_app.pr_public_surface_published`, the single retention-EXEMPT audit event type, so they are lifetime and never shrink -- while `reversal_*` rows prune with the rest of audit_events at 90 days. Pairing an immortal denominator with a 90-day numerator makes `1 - reversed/decided` drift toward 100% as the ledger ages, independent of real reversal behavior. Confirmed live after #9718 deployed: byProject still published 100% for all three repos on 2377/602/508 reviewed with 0 reversals, because the reversal signal IS observable on that deployment -- the gate passed and the ratio was still meaningless. The accuracy denominator is now a second, retention-windowed disposition query that shares BOTH of the numerator's bounds: own-ledger only, and inside audit_events' retention window. `reviewed`/`merged`/`closed` keep publishing lifetime volume, which is measured and correct. That display-vs-denominator split is the same shape #7449 established for the Orb fold and #9718 carried into the weekly trend; the #7449 snapshot it supersedes is removed rather than left dead, with its reasoning folded into the new comment. Boundary case, stated rather than hidden: a PR published before the window but reversed inside it contributes to the numerator and not the denominator. accuracyPct's existing clamp already handles that (it is the same shape as the reopened-auto-close case its comment describes), and the alternative -- an unbounded denominator -- is the bug being fixed. Refs #9676 * docs(stats): retarget the global accuracyPct comment at the variables that still exist Review catch on #9768. The comment above the global accuracyPct call site still pointed at "the ownLedgerMerged/ownLedgerClosed snapshot above the Orb fold" -- variables this PR removed and replaced with windowedMerged/windowedClosed. A comment referring a reader to a symbol that no longer exists is worse than none: it sends them looking for context that was deliberately relocated. Retargeted at the windowed disposition query, and widened while there. The old text described ONE of the two bounds the pairing needs (same population, so the fleet fold cannot inflate it); it now names both, since this PR added the second (same retention window, so an immortal denominator cannot outlive its numerator). `ownLedgerReviewed`/`ownLedgerMinutes` are untouched -- different variables, both still live, feeding the minutes-saved calculation.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…s something again (#9775) After #9718 and #9768 the own-ledger weekly trend is correct and, for recent weeks, correctly null: the hosted Worker does not execute reviews, so that ledger is frozen while Orb volume keeps growing. A column of nulls beside a rising volume column is honest and tells a reader nothing about how the gate behaves today. orb_signals already carries everything needed, with no new ingest, no new column and no new secret: self-host runtimes export gate_verdict/outcome/reversal_flag per PR, the hosted side validates and stores them, and computeFleetAnalytics already turns them into the 90-day headline. This buckets the SAME rows weekly. Two deliberate non-choices, both of which would have reintroduced the bug class this surface is being corrected for: - It is a SEPARATE series, never blended with the own-ledger one. orb_signals is keyed by per-instance HMACs of repo/PR; audit_events and orb_pr_outcomes use raw owner/repo#number. The populations cannot be joined, so a reversal from one can never be attributed to a decided row in the other, and one number over a mixed denominator would be exactly the defect #9676 reports. - It does NOT use 1 - reversalRate. #8820 established decisionAccuracy as the fleet estimand because the reversal formula divides by holds -- deferrals that cannot be right or wrong -- and misses mispredictions carrying no reversal marker. A weekly series on a different estimand than the headline directly above it would make the page disagree with itself. The weekly fold calls foldInstance rather than reimplementing the confusion matrix. That function is already exported for the federated bundle with a doc comment warning that a second copy makes comparisons apples-to-oranges, and the accounting is subtle: policy_action rows are excluded from scoring but still count as activity, and a superseded close is disconfirmed exactly like a reopen. Only registered instances count, matching computeFleetAnalytics' own trust gate -- the ingest is open, so a stranger's signals must not move a published number until a human opts them in. There is a test pinning that. Refs #9676
This was referenced Jul 29, 2026
orb_signals grows unbounded, and adding retention would silently corrupt two existing surfaces
#9783
Closed
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…ed PR (#9793) Live after both #9718 and #9768 deployed: byProject still published 100% for all three repos, on 2377/602/508 reviewed PRs with zero reversals. Neither earlier fix could have caught it. Both addressed the WINDOW the denominator covers -- #9718 withheld the ratio where a reversal was unobservable, #9768 bounded an immortal denominator to its numerator's retention -- and the defect is WHICH PRs belong in it. A reversal is by definition a human overturning an engine auto-action; recordReversalSignals only records one against a PR the engine merged or closed. The denominator was `github_app.pr_public_surface_published`: every PR that got a review surface, including the many the engine only commented on, held, or advised. None of those could produce a reversal, so each one pushes 1 - reversed/decided toward 100% for reasons unrelated to gate quality. Both surfaces now divide by the distinct PRs this deployment auto-actioned in the window -- same event types, same outcome filter, same dry-run exclusion that loadReversalDayRows already applies when anchoring the numerator, so the two halves of the ratio are finally drawn from one population. The weekly trend had the same mismatch, masked only because its own-ledger series currently reports null for recent weeks; fixing one and not the other would have left two definitions of "decided" on one page. Volume columns are untouched: reviewed/merged/closed keep publishing lifetime and fleet-folded counts, which are measured and correct. Only the ratio's denominator narrows. #9718's observability probe is REMOVED rather than left beside the thing that supersedes it. No auto-actions now means decided is 0 and the answer is null by construction -- a structural guarantee, where the probe was a heuristic that could disagree with the denominator sitting next to it. Six fixtures were made faithful rather than the assertion relaxed: each modelled a decided PR with no auto-action, which is not a reachable state. The clearest is the revert-PR regression, which recorded a reversal_reverted with no agent.action.merge -- a PR cannot be reverted unless the engine merged it. Closes #9792
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/fairnessshowed 100% accuracy for every repository and all eight weeks of the trend, while the headline read 94.6%. The 100%s are structural zeros, not measurements.recordReversalSignalshas exactly one call site, insidehandlePullRequestWebhookEvent(src/queue/processors.ts:7039), reached only via thegithub-webhookjob — a member ofREVIEW_EXECUTION_JOB_TYPES, which a runtime that does not execute reviews acks-and-drops (src/index.ts:61-75). Soreversedis pinned at 0 while merged/closed keep growing, and1 - 0/Nrenders as a flawless score over a numerator that can never move. The repo already recorded the empirical outcome —src/review/reversal-superseded.ts:3-4: "verified in production: zero reversal events ever."1. Accuracy is withheld when a reversal is not observable
accuracyPctnow returnsnullunless a reversal is observable on this deployment. A recorded reversal proves it outright — and short-circuits the probe, so no deployment that has ever overturned an auto-action pays a query. Otherwise there must be at least one terminal auto-action (agent.action.close/agent.action.merge) in the retained window for a reversal to attach to.This is the discipline the surface already applies, extended to the case where the numerator's writer, not just its sample, is absent: a sparse rule's precision is null (
PUBLIC_PRECISION_MIN_DECIDED), andgamingFlagsCaughtis null below three instances (src/orb/analytics.ts:343). Volume columns are measured directly and still publish.2. The weekly trend's denominator asymmetry
merged/closedfolded in registered-Orb rows (public-accuracy-trend.ts:181-182) whilereversedstayed own-ledger-only — the Orb aggregate has no reversal concept — so the denominator grew with every newly registered install while the numerator did not. #7449 fixed exactly this for the lifetime figure (public-stats.ts:463-470) and it was never carried across to the trend. Accuracy now divides the own-ledger pairing; displayed volume still includes the fleet.3. UI
basis. orb(public-stats): fleetAccuracy has no minimum instanceCount — at N=1 it publishes one operator's self-report as a fleet aggregate, and the anti-gaming detector it cites cannot fire #9168 computes"single_instance_self_report"precisely so one operator's self-reported number is not read as fleet corroboration — and the page dropped that field entirely, along withdecidedCountandaccuracyCiPct. It now reads: "Self-reported by that single instance, not corroborated across operators (5,225 decided, 95% CI 93.9–95.2%)."decisionAccuracy(Homepage hero metrics are wrong: decision accuracy pegged at 100%, AI-work-reused renders no value #8820) — two different quantities. It now explains both and why they can differ.Not in this PR
ai_consensus_defectandslop_gate_scorepublish byte-identicaldecided: 460, confirmed: 287because the slop backfill copies the AI-consensus corpus's labels verbatim (scripts/backfill-slop-corpus.ts:51-58). That is backfilled row data, not a code path — filtering backfilled rows out of the public precision is a separate change with its own product question. Tracked on #9676.Refs #9676
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
test:coverage:vitest run --coverageoverpublic-stats.test.ts,public-accuracy-trend.test.tsandpublic-stats-route.test.ts, restricted to the two changed modules — 98.4% branch overall, with the single uncovered line (public-stats.ts:564, a pre-existingJSON.parsecatch) outside this diff, socodecov/patch(changed lines only) is clean. Every new branch is exercised on both sides: observable/unobservable, the known-reversal short-circuit, the probe's true/false arms, its fail-safe throw path, and the own-ledger-vs-Orb denominator split.npm run ui:testgreen (643 passed, +3 new).apps/**is outside Codecov's scope by config, so these are correctness tests rather than coverage.test:workers,build:mcp,test:mcp-packnot run locally — this diff touches neither the Workers pool nor the MCP package. Left to CI.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.No auth/CORS/session behavior changes.
ui:openapi:checkconfirms the OpenAPI artifact is unchanged —accuracyPctwas already nullable on the wire, so this narrows when a value is present, not the response shape. That nullability is why the change degrades safely against an older deployed frontend.UI Evidence
Captured at 1280×800,
deviceScaleFactor: 2, dark build. Both states render through the real component against the same captured production payload (/v1/public/stats, fetched live), with only the fields this PR changes differing — so the comparison isolates the rendering change rather than mixing in unrelated live drift. The deployed API predates #9168'sbasis, sobasisis present in both payloads to keep that deploy skew out of the comparison.basisdisclosureNotes
This does not restore the reversal signal — it stops the page claiming a measurement it cannot make. Getting real reversal-grounded accuracy back on the hosted surface means either running the reversal writer on that runtime or ingesting reversals from self-host instances the way outcomes already are; both are architectural and tracked on #9676.