Skip to content

fix(stats): measure accuracy over auto-actioned PRs, not every reviewed PR - #9793

Merged
JSONbored merged 1 commit into
mainfrom
fix/accuracy-denominator-auto-actions
Jul 29, 2026
Merged

fix(stats): measure accuracy over auto-actioned PRs, not every reviewed PR#9793
JSONbored merged 1 commit into
mainfrom
fix/accuracy-denominator-auto-actions

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Live after both #9718 and #9768 deployed:

byProject accuracy : [100, 100, 100]     # on 2377 / 602 / 508 reviewed, 0 reversals
totals.accuracyPct : 100

Neither earlier fix could have caught this. 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. The remaining defect is which PRs belong in it.

A reversal is by definition a human overturning an engine auto-action; recordReversalSignals only ever records one against a PR the engine merged or closed. The denominator was github_app.pr_public_surface_publishedevery 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.

Closes #9792

The fix

Both surfaces now divide by the distinct PRs this deployment auto-actioned in the window — same event types, same outcome = 'completed' filter, same dry-run exclusion loadReversalDayRows already applies when anchoring the numerator. 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 a single page — the exact thing this surface has been corrected for.

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, not 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.

Validation

Check Result
public-stats.ts 39 changed lines, 0 uncovered, 0 partial branches
public-accuracy-trend.ts 55 changed lines, 0 uncovered, 0 partial branches
Backend suites 63 tests green
UI suites 1052 green
typecheck · ui:typecheck · ui:openapi:check · coverage-boltons · validate:no-hand-written-js · git diff --check clean

New regressions pin the live defect directly: a repo with three reviewed-and-merged PRs but no auto-actions now publishes null, not 100% — with reviewed: 3 and merged: 3 still published, since the volume is real. A second pins that a dry-run auto-action never counts, because the numerator's anchor already excludes them and a denominator that didn't would inflate against 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: it recorded a reversal_reverted with no agent.action.merge, and a PR cannot be reverted unless the engine merged it. The retention-window test from #9768 was also strengthened — its four ancient PRs are now auto-actioned too, so it proves the window excludes them rather than merely that they were never actioned.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section 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.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

No auth/CORS/session change and no new data in the response — accuracyPct was already nullable on the wire, so ui:openapi:check passes unchanged and an older deployed frontend degrades correctly. No UI file is touched: #9718 already shipped the copy explaining a withheld accuracy, and it now fires for the right reason, hence no UI Evidence table. The queries read audit_events aggregate counts only — no target keys, repos or PR content reach the response, and the existing INVARIANT: the public payload never carries target keys… test still passes.

Expected effect in production

The hosted Worker does not execute reviews (#9676), so recent auto-actions are absent or few. byProject should go null or drop to a small honest number instead of asserting 100% across 3,487 PRs. That is the point: an unmeasurable ratio should read as unknown, not as perfect.

…ed PR

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
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-29 10:16:58 UTC

5 files · 1 AI reviewer · no blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR narrows the accuracy denominator from every published-surface PR to only those PRs the deployment actually auto-actioned (agent.action.close/merge), fixing the real defect where PRs merely commented on were inflating the accuracy ratio toward 100%. The same query shape (event types, outcome filter, dry-run exclusion) is reused consistently between public-stats.ts and public-accuracy-trend.ts, and the old loadReversalObservability probe is correctly removed as redundant since a zero auto-action denominator now naturally yields null. Tests are updated to model the auto-action population explicitly and include solid regression coverage for the exact production symptom (100% accuracy over thousands of reviewed PRs with zero reversals) and dry-run exclusion.

Nits — 6 non-blocking
  • src/review/public-stats.ts and public-accuracy-trend.ts duplicate the same target_key-parsing and dry-run-exclusion SQL fragment in three places (loadAutoActionDayRows, loadReversalDayRows, and the windowed disposition query) — consider extracting a shared SQL fragment/CTE the way PUBLISHED_PR_KEYS already does.
  • The magic number '9792' appears only in comments referencing the issue, not as a named constant — fine as-is, but the external brief's note is a non-issue since it's just an issue reference in a comment.
  • src/review/public-stats.ts has grown to ~636 lines; consider splitting the disposition/accuracy queries into a separate module if further growth is expected, though this PR's diff itself is a net reduction (-52/+39) so it isn't the cause of the size.
  • Extract the repeated 'agent.action.close/merge, outcome=completed, dry-run excluded, single-# target_key' predicate into a shared SQL snippet or CTE reused by loadAutoActionDayRows, loadReversalDayRows, and the getPublicStats windowed query.
  • Consider whether loadAutoActionDayRows' COUNT(DISTINCT act.target_key) and the windowed disposition query's join could be unified into one query to reduce duplicate D1 round-trips per request.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9792
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: 14 registered-repo PR(s), 13 merged, 361 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 361 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The diff replaces the published-surface-based denominator with a denominator derived from distinct PRs auto-actioned (agent.action.close/merge, outcome=completed, dry-run excluded) on both the live public-stats surface and the weekly trend, matching the numerator's population as the issue requested, and removes the now-redundant #9718 observability probe in favor of the structural null-when-zero g

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 14 PR(s), 361 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 1 step in the Signals table above.
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.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: ai_review_inconclusive
  • config: f0c0b55f247be6ed87d84097f0c48ae7c811974421c6e28fe389c7c80ac032db · pack: oss-anti-slop · ci: passed
  • record: b6f3446fa3f5a69bcbce460487c44f99835dd7870ffabf0ae3a60e848fe8dc67 (schema v5, head d34cd41)

🟩 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.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.48%. Comparing base (0fa52ed) to head (d34cd41).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9793      +/-   ##
==========================================
- Coverage   90.30%   89.48%   -0.82%     
==========================================
  Files         913      913              
  Lines      113595   113587       -8     
  Branches    26963    26958       -5     
==========================================
- Hits       102584   101646     -938     
- Misses       9682    10851    +1169     
+ Partials     1329     1090     -239     
Flag Coverage Δ
backend 94.09% <100.00%> (-1.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/public-stats.ts 97.41% <ø> (-0.19%) ⬇️
src/services/public-accuracy-trend.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 29, 2026
@JSONbored
JSONbored merged commit e1c092b into main Jul 29, 2026
7 checks passed
@JSONbored
JSONbored deleted the fix/accuracy-denominator-auto-actions branch July 29, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fairness accuracy still reads 100% after #9718 and #9768: the denominator counts reviewed PRs, not auto-actioned ones

1 participant