metrics(ledger): re-evaluation counts and per-author-class review-parity rollups - #9877
Open
JSONbored wants to merge 5 commits into
Open
metrics(ledger): re-evaluation counts and per-author-class review-parity rollups#9877JSONbored wants to merge 5 commits into
JSONbored wants to merge 5 commits into
Conversation
Contributor
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-29 20:07:34 UTC
Review summary Nits — 6 non-blocking
CI checks failing
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. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. 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. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 059394c | Commit Preview URL Branch Preview URL |
Jul 29 2026, 07:44 PM |
Bundle ReportChanges will increase total bundle size by 18.49kB (0.23%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
…ity rollups (#9743) Two questions this deployment could only answer with an ad-hoc query now have standing numbers on /v1/public/stats: how often a verdict is re-evaluated and why, and whether a PR faces the same scrutiny regardless of who wrote it. Everything is computed from decision_records -- the anchored ledger -- and nothing else, so an outsider holding the export can recompute every figure. That constraint is why findingsCount is now recorded ON the decision record (v6, migration 0205): findings existed only in ai_review_cache, which is keyed for reuse rather than anchored and is pruned independently, so a fairness number counted from it would have been unverifiable and quietly wrong once the cache turned over. Author class is GitHub's own author_association, not a list this project maintains. That predicate was spelled out at four independent call sites -- one lower-cased, one under a different name -- so it now lives once in github/author-association.ts and they all read it. The definitions that are easy to get wrong are stated beside the code and in the verifier walkthrough: verdicts counts EVALUATIONS not PRs; findingsPerPr excludes verdicts that recorded no count rather than averaging them in as zero, and publishes the coverage it was earned at; an unrecorded association is its own `unknown` bucket rather than folded into either side, because folding it would bias the exact comparison being published; and a rate over an empty window is null, never 0.
Both series #9743 computes now render on /fairness, with the methodology stated where the numbers are rather than only in the docs. The zero states are the point. A window with no verdicts renders as a MEASURED zero with its own dates -- "not missing data" in as many words -- and "no re-evaluations recorded" is a separate, separately-worded state from "no verdicts recorded", because they are different facts and a reader cannot tell them apart otherwise. A mean with no basis reads as insufficient data, never as 0, and every published mean carries the count it was earned at beside it. The payload shape is added to the contract's zod schema, so the UI type is DERIVED rather than hand-kept: adding the block made both existing fixtures fail to compile, which is exactly the guard #9282/#9521 put there.
…ine package The gate-advisory TWIN carried a FIFTH copy of the OWNER/MEMBER/COLLABORATOR predicate, and the engine is standalone -- it cannot import out of src/. So the definition lives in @loopover/engine and src/github/author-association.ts re-exports it, the same shape settings/pr-type-label.ts already uses for exactly this reason. Both twins now read one definition.
persistDecisionRecord bound record.findingsCount raw. A caller that assembles a DecisionRecord literal rather than going through buildDecisionRecord leaves it undefined, D1 refuses undefined as a bind, and the insert threw straight into the outer catch -- so the row silently never appeared and the ledger simply had fewer decisions in it, with nothing surfaced anywhere. Caught by proof-summary's own fixtures, which build records that way. The bind coalesces to null now (the column is nullable and undefined is never a valid bind), with a regression test that fails if the coalesce is removed.
… ENGINE's own suite Same cause as the sibling fix on #9847. packages/loopover-engine/src/** is credited by two Codecov uploads, and the "engine" flag is fed by the package's own node:test suite -- so a module that only the root vitest suite exercises reads as uncovered there. author-association.ts moved INTO the engine in this PR, which makes every one of its lines new to that flag. The predicate is now behaviour-tested in the engine suite, including through buildPullRequestAdvisory, since the gate-advisory twin is one of its callers and is where the fifth duplicate copy lived.
JSONbored
force-pushed
the
feat/reevaluation-parity-rollups-9743
branch
from
July 29, 2026 19:40
3fbd4a2 to
059394c
Compare
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.
Closes #9743. Closes #9744. Completes epic #9741.
Problem
Two questions this deployment could only answer with an ad-hoc query: how often is a verdict re-evaluated, and why, and does a PR face the same scrutiny regardless of who wrote it. #9742 made every repeat verdict declare a reason; this counts them, and puts the author-class comparison beside it.
Everything is computed from the anchored ledger — which forced one schema change
decision_recordsand nothing else, so an outsider holding the export can recompute every published figure. That constraint is load-bearing rather than stylistic, and it is whyfindingsCountis now recorded on the decision record (schema v6, migration 0205): findings existed only inai_review_cache, which is keyed for reuse rather than anchored and is pruned independently. A fairness number counted out of a cache is unverifiable, and would have gone quietly wrong the first time the cache turned over.Author class is GitHub's, not ours
pull_requests.author_association— mechanical, so a permissions change upstream changes the class with no code change here, which is what #9743 asked for. That predicate was already spelled out at four independent call sites (the mention-command gate, the settings preview, the advisory rules, and local-branch's owner check — one lower-cased, one under a different name). It now lives once insrc/github/author-association.tsand they all read it.The definitions that are easy to get wrong
Stated beside the code, in the verifier walkthrough, and on the page itself:
verdictscounts evaluations, not pull requests. A repeat evaluation of one head SHA is its own ledger row — which is exactly whatreviewsPerPrdivides out.findingsPerPrexcludes verdicts that recorded no count, rather than averaging them in as zero, and publishesfindingsBasis— the coverage the mean was earned at. A mean over 3 of 400 verdicts is not the same claim as a mean over 400.unknownbucket, never folded into either side. Folding it would bias the exact comparison being published.null, never0. "Nothing was held" and "nothing was measured" are different claims.shareOfVerdictsPctis a share of all verdicts, not of re-evaluations — the latter always sums to 100% and says nothing about how often re-evaluation happens at all./fairness (#9744)
Both series render with their methodology where the numbers are. The zero states carry the weight: an empty window renders as a measured zero with its dates, and "no re-evaluations recorded" is a separately-worded state from "no verdicts recorded" because they are different facts.
The payload shape went into the contract's zod schema, so the UI type is derived. Adding the block made both existing UI fixtures fail to compile — which is precisely the guard #9282/#9521 put there, doing its job.
Validation
v8 ignoresuppressions.ui:typecheck,ui:build,ui:lint(0 errors) clean.db:migrations:check(contiguous 0001..0205),db:schema-drift:check,ui:openapi:check,contract:api-schemas:check,ui-derived-types:check,docs:drift-check,dead-exports:check,import-specifiers:checkall clean.