Skip to content

feat(review-stats): add finding acceptance-rate signal to the maintainer stats feed (#1967)#5127

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
carlh7777:feat/finding-acceptance-rate
Jul 12, 2026
Merged

feat(review-stats): add finding acceptance-rate signal to the maintainer stats feed (#1967)#5127
JSONbored merged 1 commit into
JSONbored:mainfrom
carlh7777:feat/finding-acceptance-rate

Conversation

@carlh7777

Copy link
Copy Markdown
Contributor

Summary

Adds the finding acceptance-rate signal to the maintainer stats feed — the one metric #1967 calls out as missing: "no acceptance-rate metric. Add a finding acceptance-rate signal (did the contributor act on a finding → merged)."

Of PRs whose gate raised a blocking finding (a gate_decision of hold or close — the gate did not clear the PR to merge), it measures how often the contributor acted on it and the PR still merged. The realized pr_outcome (merged vs closed) is the answer key, so acceptance is measurable with zero manual labeling — the maintainer-ROI "our review feedback gets acted on X% of the time" signal.

This follows the existing metric-adder pattern in src/review/stats.ts exactly — the same shape as cycle-time (#2194), calibration bins (#2192), and review-effort (#2155):

  • FindingAcceptanceAggregate type + EMPTY_FINDING_ACCEPTANCE constant.
  • aggregateFindingAcceptance(samples) — a pure fold (addressed/unaddressed/acceptanceRate).
  • computeFindingAcceptance(env, opts) — a fail-safe D1 reader over review_audit, reusing the same gate_decisionpr_outcome join the gate-eval already uses. A PR is "flagged" if it ever got a hold/close decision in the window (DISTINCT target_id) — so held-then-fixed-then-merged still counts as addressed — joined to the latest pr_outcome per target.
  • findingAcceptance field wired into computeStats / StatsPayload.

Scoped deliberately to the stats.ts feed (which #1967 names). Surfacing it on the operator-dashboard payload and a dashboard card is deferred to keep this diff narrow and single-purpose.

Closes #1967

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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 (Closes Maintainer analytics dashboard (gate precision, calibration, acceptance rate) #1967) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally (focused): npx vitest run test/unit/stats.test.ts — 52 pass, v8 coverage 100% stmts/lines/funcs on src/review/stats.ts, 100% on every changed line and branch (the sole uncovered branch is pre-existing timingSafeEqual, untouched by this diff). Will run full npm run test:ci + npm audit before push.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and boundaries (pure-fold extremes, real-D1 join incl. clean-merge exclusion + no-outcome exclusion + latest-outcome rn=1, fail-safe reject, null-results ?? [], window default/clamp).

If any required check was skipped, explain why:

Backend-only change (src/review/stats.ts + its unit test). No workflow, MCP, OpenAPI, or UI surface is touched, so actionlint, build:mcp/test:mcp-pack, ui:*, and ui:openapi:check are N/A. handleStats returns a raw JSON feed (not OpenAPI-typed), so no schema regeneration is required. Full test:ci + npm audit will run before push.

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. (N/A — no auth/session/CORS surface touched; the feed's existing bearer gate is unchanged.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — the stats feed is a raw bearer-gated JSON response, not OpenAPI/MCP-typed.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
  • Visible UI changes include a UI Evidence section below. (N/A — no visible UI changes.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A.)

UI Evidence

N/A — no UI/frontend/docs changes in this PR.

Notes

  • Metric definition is transparent: flagged = addressed + unaddressed, acceptanceRate = addressed / flagged (3 dp), null when nothing was flagged.
  • Fully fail-safe (mirrors computeCycleTimeAggregate): any D1 read error or a stats-window without flagged PRs degrades to EMPTY_FINDING_ACCEPTANCE, never throwing.
  • Surfacing this on the operator-dashboard payload + a dashboard card is intentionally deferred past this suggest-the-signal slice.

…es (JSONbored#1967)

Introduces the FindingAcceptanceAggregate interface to track the acceptance rate of PRs flagged with blocking findings (hold/close). Implements aggregateFindingAcceptance function to compute the acceptance metrics from flagged PR outcomes. Updates computeStats to include finding acceptance data and adds corresponding tests to validate the new functionality.
@carlh7777 carlh7777 requested a review from JSONbored as a code owner July 11, 2026 21:03
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 11, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.32%. Comparing base (a416632) to head (6e0b0be).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5127   +/-   ##
=======================================
  Coverage   94.32%   94.32%           
=======================================
  Files         471      471           
  Lines       39821    39834   +13     
  Branches    14533    14535    +2     
=======================================
+ Hits        37560    37573   +13     
  Misses       1583     1583           
  Partials      678      678           
Flag Coverage Δ
shard-1 46.33% <14.28%> (-0.02%) ⬇️
shard-2 34.40% <14.28%> (-0.26%) ⬇️
shard-3 31.26% <14.28%> (+0.28%) ⬆️
shard-4 32.73% <14.28%> (-0.15%) ⬇️
shard-5 33.44% <100.00%> (-0.21%) ⬇️
shard-6 45.16% <14.28%> (+0.29%) ⬆️

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

Files with missing lines Coverage Δ
src/review/stats.ts 99.18% <100.00%> (+0.09%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 11, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-11 21:10:10 UTC

2 files · 2 AI reviewers · no blockers · readiness 100/100 · CI green · unstable

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This adds a finding-acceptance-rate aggregate to the maintainer stats feed, following the exact established pattern of the existing cycle-time/review-effort metrics in src/review/stats.ts: a pure fold (aggregateFindingAcceptance), a fail-safe D1 reader (computeFindingAcceptance) with the same days-clamp (90 default, 730 max) and try/catch-to-EMPTY pattern as computeCycleTimeAggregate, and wiring into computeStats/StatsPayload. The SQL correctly uses DISTINCT target_id over hold|close gate_decision rows joined to the latest (ROW_NUMBER rn=1) pr_outcome per target, which correctly handles the held-then-fixed-then-merged case demonstrated in the PR#3 test fixture. Tests cover the pure fold at both extremes, the empty case, a real-D1 integration test exercising the actual join logic, and the fail-safe/undefined-results/window-clamp paths — this is solid coverage of a narrowly-scoped, well-targeted addition.

Nits — 5 non-blocking
  • src/review/stats.ts's FINDING_ACCEPTANCE_SQL introduces several new unexplained numeric/string literals (90, 730, 86_400_000) that mirror computeCycleTimeAggregate's existing clamp constants but are duplicated rather than shared — consider extracting a shared STATS_WINDOW_DEFAULT_DAYS/MAX_DAYS constant used by both.
  • src/review/stats.ts is now ~483 lines, over the repo's apparent 400-line file-size convention; if more per-issue metrics keep landing here, consider splitting metric aggregators into their own module.
  • The po CTE and flagged CTE both filter on `created_at >= ?` using the same fromIso — worth a one-line comment noting this relies on pr_outcome always postdating its gate_decision (already true in practice) so no flagged PR's outcome is silently dropped by the window.
  • Extract the days-clamp logic (Math.min(opts.days, 730) / default 90) shared between computeCycleTimeAggregate and computeFindingAcceptance into a small shared helper to avoid drift if one is tuned later.
  • Consider adding a brief doc/dashboard follow-up issue reference since the PR explicitly defers surfacing this on the operator dashboard — makes the deferred scope traceable.
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review
Signal Result Evidence
Code review ✅ No blockers 2 reviewers, synthesized
Linked issue ✅ Linked #1967
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: 182 registered-repo PR(s), 114 merged, 17 issue(s).
Contributor context ✅ Confirmed Gittensor contributor carlh7777; Gittensor profile; 182 PR(s), 17 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The diff adds one well-scoped, well-tested metric that directly closes the specific gap named in #1967, following the codebase's existing metric-adder convention exactly rather than introducing new patterns or surface area.
Linked issue satisfaction

Partially addressed
The PR adds the acceptance-rate signal (aggregateFindingAcceptance/computeFindingAcceptance) into the stats.ts feed exactly as requested, but the issue's deliverables also call for dashboard views, an export endpoint, and docs, none of which are present in this diff (explicitly deferred per the PR description).

Review context
  • Author: carlh7777
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, TypeScript, Cuda, HTML, MDX, Rust
  • Official Gittensor activity: 182 PR(s), 17 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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

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

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@JSONbored JSONbored merged commit d1cb4e5 into JSONbored:main Jul 12, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maintainer analytics dashboard (gate precision, calibration, acceptance rate)

2 participants