Skip to content

fix(review): derive the proof-page accuracy from real reversals and surface ledger exclusions - #10178

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/proof-summary-real-accuracy-ledger-10012
Jul 31, 2026
Merged

fix(review): derive the proof-page accuracy from real reversals and surface ledger exclusions#10178
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/proof-summary-real-accuracy-ledger-10012

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

src/review/proof-summary.ts renders the unauthenticated per-repo proof page. Two of its published claims were structurally unable to be anything but perfect.

1. confirmed could never differ from decided, so accuracy was always exactly 1

SUM(CASE WHEN action IN ('merge','close') AND reason_code NOT LIKE 'reversal%' THEN 1 ELSE 0 END) AS confirmed

decision_records.reason_code is written once at decision time from deriveDecisionReasonCode / defaultDecisionRecordReasonCode, neither of which can produce a reversal… value, and nothing ever UPDATEs it. Reversals are recorded as separate audit_events rows (reversal_reverted / reversal_reopened / reversal_superseded), which this query never touched. So confirmed === decided for every repo, always, and buildProofAccuracy published accuracy: round3(confirmed / decided) = 1 with a Wilson interval hugging 1 — the exact denominator-choice failure the codebase already diagnosed once on public-stats.ts ("STILL 100% for all three repos").

2. A ledger with declared waivers or pruned preimages still badged plain "verified"

verifyDecisionLedger reports prunedRecords, waivedContentMismatches and waivedUnchainedRecords, but the proof page's verifyLedger dependency signature dropped them — so buildProofLedgerStatus could only ever produce { state: "verified" } and buildProofBadgeMessage rendered "verified · anchored". On an instance carrying the LOOPOVER_LEDGER_*_WAIVER declarations those parsers exist to support, the most public surface rendered exactly the silence the internal verifier refuses to.

The fix

Accuracy: count confirmed as merge/close decisions with no reversal_* audit_events row for their <repo>#<pull> (anti-join, mirroring public-rule-precision's treatment of the same event types), in its own section(...). A failing reversal read degrades to insufficient_data (confirmed = null) rather than asserting confirmed === decided. decided stays the count of action IN ('merge','close').

Ledger: carry prunedRecords / waivedContentMismatches / waivedUnchainedRecords onto the verified status (and the two verifyLedger dependency signatures). buildProofBadgeMessage now returns "verified · N excluded" (N = the sum) with buildProofBadgeColor returning the neutral "#9e9e9e" whenever that sum is > 0; a sum of 0 keeps today's exact strings/colours byte-identically.

Unchanged: PROOF_MIN_DECISIONS, PROOF_SAMPLE_RECORDS, round3, buildProofAccuracy's published/insufficient_data shape, buildProofAnchorStatus, PROOF_BOUNDARY_STATEMENT, the allowlist-by-name privacy discipline, and the empty/broken/unavailable ledger states.

Tests (test/unit/proof-summary.test.ts)

  • REGRESSION: 25 merge/close decisions with 2 reversal_reverted rows → accuracy: { state: "published", decided: 25, confirmed: 23, accuracy: 0.92 } (fails on main, which publishes 1).
  • 25 decisions with zero reversals still publishes accuracy: 1 — the perfect case stays expressible.
  • A failing reversal read degrades to insufficient_data, never a fabricated rate.
  • buildProofBadgeMessage / buildProofBadgeColor for a verified ledger with waivedUnchainedRecords: 231"verified · 231 excluded" / "#9e9e9e"; the zero-excluded case keeps "verified · anchored" / "#3fb950".
  • The existing 24 tests (fixtures updated for the widened verify/ledger shape) all still pass.

Validation

  • Diff coverage on src/review/proof-summary.ts is 100% line and branch.
  • npm run typecheck clean (the production verifyLedger binding forwards verifyDecisionLedger's full result, which already carries the three counts); engine-parity:drift-check, ui:openapi:check and ui-derived-types:check all pass; dead-exports:check clean; the suite (29 tests) green.
  • git diff --check clean; no migration (the reversal signal is already in audit_events).

Closes #10012

…urface ledger exclusions

The public proof page published two figures that could never be anything but perfect.

1. accuracy was always 1. confirmed was SUM(action IN (merge,close) AND reason_code
   NOT LIKE 'reversal%'), but decision_records.reason_code is written once at decision
   time from two derivations, neither of which can produce a 'reversal…' value, and
   nothing updates it — reversals are separate audit_events rows. So confirmed === decided
   for every repo and buildProofAccuracy published accuracy = round3(confirmed/decided) = 1
   with a Wilson interval hugging 1, the exact denominator-choice failure the codebase
   already diagnosed once on public-stats. Count confirmed as merge/close decisions with
   NO reversal_reverted/reversal_reopened/reversal_superseded audit row for their
   <repo>#<pull>, in its OWN section: a failing read degrades to insufficient_data (null
   confirmed) rather than asserting confirmed === decided.

2. a ledger with declared waivers or pruned preimages still badged plain 'verified'.
   verifyDecisionLedger reports prunedRecords / waivedContentMismatches /
   waivedUnchainedRecords, but the proof page's verifyLedger dependency dropped them, so
   buildProofLedgerStatus could only ever render 'verified · anchored'. Carry the three
   counts onto the verified status; buildProofBadgeMessage now returns 'verified · N excluded'
   (N = their sum) with the neutral colour when the sum is > 0, and today's exact strings
   and colours when it is 0.

Unchanged: PROOF_MIN_DECISIONS, PROOF_SAMPLE_RECORDS, round3, buildProofAccuracy's
published/insufficient_data shape, the anchor status, the empty/broken/unavailable ledger
states, and the allowlist-by-name privacy discipline.

Closes JSONbored#10012
@shin-core
shin-core requested a review from JSONbored as a code owner July 31, 2026 11:05
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 11:16:15 UTC

2 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a solid, well-scoped bug fix that replaces two structurally-broken proof-page claims: the `confirmed`/`decided` accuracy metric (which was mathematically forced to 1.0 because `reason_code` can never contain a reversal value) is now derived via an anti-join against real `reversal_*` audit_events rows, and the ledger badge now surfaces `prunedRecords`/`waivedContentMismatches`/`waivedUnchainedRecords` instead of collapsing them into a bare 'verified'. The failure-mode discipline is careful — a failed reversal read yields `confirmed: null`, which `buildProofAccuracy` degrades to `insufficient_data` rather than fabricating a rate — and the added tests exercise both the happy path (2/25 reversed → 0.92) and the degraded path. The one gap worth noting is that the public-facing React component (`public-proof-page.tsx`) still declares the old `ProofLedgerStatus` shape and never renders the new exclusion counts, so the stated goal of 'surfacing ledger exclusions' is only realized in the badge string, not the JSON page's UI.

Nits — 5 non-blocking
  • src/review/proof-summary.ts's new anti-join keys reversal audit_events only on `repo_full_name#pull_number`, not on head SHA — if `decision_records` can hold multiple rows per pull_number (e.g. re-decided after a new push), a single reversal event would mark every historical decision row for that pull as unconfirmed, not just the one it actually reverses; worth confirming whether decision_records enforces one row per pull_number or this is an existing multi-row model.
  • apps/loopover-ui/src/components/site/public-proof-page.tsx's local `ProofLedgerStatus`/`ProofSummary` types were not updated to include `prunedRecords`/`waivedContentMismatches`/`waivedUnchainedRecords`, so the UI page itself has no way to display the exclusion counts even though the API now returns them.
  • The `/* v8 ignore next */` comment on `confirmedRow.confirmed ?? 0` in src/review/proof-summary.ts asserts COUNT(*) is never NULL, which is true for SQLite/D1 but is an unenforced assumption worth a one-line note if the driver ever changes.
  • The anti-join subquery in loadProofSummary embeds string concatenation (`d.repo_full_name || '#' || d.pull_number`) to match `target_key` — consider indexing `audit_events(target_key)` if not already indexed, since this now runs on every proof-page load.
  • Update apps/loopover-ui/src/components/site/public-proof-page.tsx's ProofLedgerStatus type and LedgerCard component to render the exclusion counts, matching the badge's new behavior.

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 #10012
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: 78 registered-repo PR(s), 60 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 78 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The diff replaces the reason_code-based confirmed count with an anti-join against reversal_* audit_events keyed by <repo>#<pull>, runs it in its own section that degrades to insufficient_data on failure, and adds prunedRecords/waivedContentMismatches/waivedUnchainedRecords to ProofLedgerStatus's verified variant and both verifyLedger signatures, with buildProofBadgeMessage now producing "verified

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 78 PR(s), 0 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.
🧪 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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.

🟩 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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.46%. Comparing base (505eda3) to head (d4bac91).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #10178       +/-   ##
===========================================
- Coverage   92.21%   80.46%   -11.75%     
===========================================
  Files         932      283      -649     
  Lines      114114    58829    -55285     
  Branches    27566     8706    -18860     
===========================================
- Hits       105229    47339    -57890     
- Misses       7583    11199     +3616     
+ Partials     1302      291     -1011     
Flag Coverage Δ
backend 100.00% <100.00%> (+4.32%) ⬆️

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

Files with missing lines Coverage Δ
src/review/proof-summary.ts 100.00% <100.00%> (ø)

... and 781 files with indirect coverage changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit c378e0a into JSONbored:main Jul 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(proof): the public proof page's accuracy is 100% by construction, and a waived/pruned ledger still badges "verified"

1 participant