Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,65 @@ describe("FairnessReportPage (#fairness-analytics)", () => {
expect(screen.getByText("2 human-reversed, lifetime")).toBeTruthy();
});

it("explains a withheld accuracy instead of letting it read as a dash-shaped mystery", async () => {
// Backend publishes null when no auto-action was ever recorded for a reversal to attach to; the page must
// say that rather than leaving a bare "—" next to a healthy-looking volume.
apiFetch.mockResolvedValue({
ok: true,
data: {
...FIXTURE,
byProject: [
{ project: "owner/repo", reviewed: 100, merged: 60, closed: 30, accuracyPct: null },
],
accuracyTrend: [
{ weekStart: "2026-07-13", merged: 30, closed: 15, reversed: 0, accuracyPct: null },
],
},
status: 200,
durationMs: 10,
});
renderWithClient(<FairnessReportPage />);

await waitFor(() => expect(screen.getByText("By repository")).toBeTruthy());
const notes = screen.getAllByText(/not measurable on this deployment, not 100%/);
expect(notes.length).toBe(2); // one under each affected table
});

it("does not show the unmeasurable-accuracy note when every accuracy is real", async () => {
apiFetch.mockResolvedValue({ ok: true, data: FIXTURE, status: 200, durationMs: 10 });
renderWithClient(<FairnessReportPage />);

await waitFor(() => expect(screen.getByText("By repository")).toBeTruthy());
expect(screen.queryByText(/not measurable on this deployment/)).toBeNull();
});

it("#9168: discloses a single-instance self-report rather than presenting it as fleet corroboration", async () => {
apiFetch.mockResolvedValue({
ok: true,
data: {
...FIXTURE,
fleetAccuracy: {
...FIXTURE.fleetAccuracy,
instanceCount: 1,
basis: "single_instance_self_report",
decidedCount: 5225,
accuracyCiPct: { lo: 93.9, hi: 95.2 },
},
},
status: 200,
durationMs: 10,
});
renderWithClient(<FairnessReportPage />);

await waitFor(() => expect(screen.getByText("Decision accuracy")).toBeTruthy());
expect(screen.getByText(/Self-reported by that single instance/).textContent).toContain(
"5,225 decided",
);
expect(screen.getByText(/Self-reported by that single instance/).textContent).toContain(
"93.9–95.2%",
);
});

it("#9068: renders the insufficient-instances state (not a fabricated zero) when gamingFlagsCaught is null", async () => {
apiFetch.mockResolvedValue({
ok: true,
Expand Down
52 changes: 47 additions & 5 deletions apps/loopover-ui/src/components/site/fairness-report-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@ import type { PublicStats } from "@/components/site/proof-of-power-stats-model";
// count, with a short methodology note. Counts only; no PR content, contributor identities, or trust scores.

const pctFmt = new Intl.NumberFormat("en", { maximumFractionDigits: 1 });
/** Why an accuracy cell reads "—". Reversal-grounded accuracy needs the deployment to have recorded the
* terminal auto-actions a reversal attaches to; where it hasn't, `1 - 0/N` would render as a flawless 100%
* over a numerator that can never move, so the backend publishes null and the page says so out loud. */
function UnmeasurableAccuracyNote() {
return (
<p className="mt-3 text-token-xs text-muted-foreground">
An accuracy of <span className="font-mono">—</span> means not measurable on this deployment,
not 100%: no auto-merge/auto-close was recorded here for a human reversal to be counted
against. The volume columns beside it are measured directly and are unaffected.
</p>
);
}

const intFmt = new Intl.NumberFormat("en");

async function fetchPublicStats(): Promise<PublicStats | null> {
Expand Down Expand Up @@ -119,6 +132,23 @@ export function FairnessReportPage() {
? `${intFmt.format(data.totals.reversed)} human-reversed, lifetime`
: "reversal-grounded, lifetime"}
</p>
{/* #9168 computes `basis` precisely so this number is not read as corroborated-across-operators
when it is one operator's own disclosed outcomes; the page used to drop the field entirely. */}
{fleetEligible && data.fleetAccuracy.basis === "single_instance_self_report" ? (
<p className="mt-2 text-token-xs text-muted-foreground">
Self-reported by that single instance, not corroborated across operators
{data.fleetAccuracy.decidedCount != null
? ` (${intFmt.format(data.fleetAccuracy.decidedCount)} decided`
: ""}
{data.fleetAccuracy.decidedCount != null &&
data.fleetAccuracy.accuracyCiPct != null
? `, 95% CI ${pctFmt.format(data.fleetAccuracy.accuracyCiPct.lo)}–${pctFmt.format(data.fleetAccuracy.accuracyCiPct.hi)}%)`
: data.fleetAccuracy.decidedCount != null
? ")"
: ""}
.
</p>
) : null}
</Card>
<Card className="p-5">
<div className="text-token-xs text-muted-foreground">Anti-gaming flags caught</div>
Expand Down Expand Up @@ -149,11 +179,15 @@ export function FairnessReportPage() {

<div className="mt-10 space-y-2 rounded-token border-hairline px-4 py-4 text-token-sm text-muted-foreground">
<p>
<span className="font-medium text-foreground">How accuracy is measured:</span> 1
minus the share of auto-merged/auto-closed PRs a human later overturned — a
bot-closed PR a contributor reopened, or a bot-merged PR undone by a separate revert
PR. Nothing here is a prediction or a self-assessment; it's counted after the fact
from what actually happened on GitHub.
<span className="font-medium text-foreground">How accuracy is measured:</span> the
headline scores the gate's own merge/close <em>decisions</em> — the share the
realized outcome confirmed, with holds excluded because a deferral to a human is not
a decision that can be right or wrong (#8820). The per-repository and weekly tables
below are a different, stricter measure: 1 minus the share of
auto-merged/auto-closed PRs a human later overturned — a bot-closed PR a contributor
reopened, or a bot-merged PR undone by a separate revert PR. Neither is a prediction
or a self-assessment; both are counted after the fact from what actually happened on
GitHub, which is also why the two can differ.
</p>
<p>
<span className="font-medium text-foreground">
Expand Down Expand Up @@ -206,6 +240,9 @@ export function FairnessReportPage() {
</tbody>
</table>
</TableScroll>
{data.byProject.some((row) => row.accuracyPct == null) ? (
<UnmeasurableAccuracyNote />
) : null}
</div>
) : null}

Expand Down Expand Up @@ -256,6 +293,11 @@ export function FairnessReportPage() {
</tbody>
</table>
</TableScroll>
{data.accuracyTrend.some(
(week) => week.merged != null && week.accuracyPct == null,
) ? (
<UnmeasurableAccuracyNote />
) : null}
</div>

{data.rulePrecision && data.rulePrecision.rules.length > 0 ? (
Expand Down
46 changes: 43 additions & 3 deletions src/review/public-stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import { validateCalibrationPayload } from "./risk-control";
import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest";
import { errorMessage } from "../utils/json";
import { retentionCutoffIsoForTable } from "../db/retention";

/** FALLBACK estimate of maintainer review/triage time saved per reviewed PR, used ONLY when the real per-PR
* average (`estimateReviewEffort`'s minutes, persisted at publish time — see `reviewEffortMinutes` in the
Expand Down Expand Up @@ -164,12 +165,47 @@ function filteredPct(reviewed: number, merged: number): number | null {
return Math.round(((reviewed - merged) / reviewed) * 1000) / 10;
}

/** Reversal-grounded accuracy over the irreversible auto-actions (merged + closed); null until there is signal. */
/** The auto-actions a reversal is recorded against. `recordReversalSignals` only ever fires for a PR this
* deployment itself auto-merged or auto-closed, so with zero of these rows in the retained window a
* `reversal_*` event cannot exist — and `1 - 0/N` is then a structural zero, not a measurement. */
export const AUTO_ACTION_EVENT_TYPES = ["agent.action.close", "agent.action.merge"] as const;

/**
* Whether a reversal is OBSERVABLE on this deployment: did it record any terminal auto-action in the window a
* reversal could still be attributed to? Reversal signals are written only by the review-execution pipeline
* (`recordReversalSignals`, reached via the `github-webhook` job), which a runtime that does not execute
* reviews acks-and-drops — so on such a runtime `reversed` is pinned at 0 forever while the merged/closed
* denominator keeps growing, and every reversal-grounded percentage converges on a fake 100%.
*
* Publishing that as accuracy is the exact failure #8820 called out for the fleet number ("the old formula
* overstated accuracy") and #7449 fixed for the global one. This is the same discipline the rest of this
* surface already applies — a sparse rule's precision is null, `gamingFlagsCaught` is null below three
* instances — extended to the case where the numerator's WRITER, not just its sample, is absent.
*/
export async function loadReversalObservability(env: Env, knownReversals = 0): Promise<boolean> {
// A recorded reversal is itself proof the pipeline that records them runs here -- no probe needed, and no
// query on the hot path for any deployment that has ever overturned an auto-action.
if (knownReversals > 0) return true;
const rows = await safeAll<{ n: number }>(
env,
`SELECT COUNT(*) AS n FROM audit_events
WHERE event_type IN (${AUTO_ACTION_EVENT_TYPES.map((type) => `'${type}'`).join(", ")})
AND created_at >= ?`,
retentionCutoffIsoForTable("audit_events"),
);
return (rows[0]?.n ?? 0) > 0;
}

/** Reversal-grounded accuracy over the irreversible auto-actions (merged + closed); null until there is signal,
* and null when a reversal is not observable at all on this deployment (see {@link loadReversalObservability}) —
* an unmeasurable quantity stays unknown rather than rendering as a perfect score. */
function accuracyPct(
merged: number,
closed: number,
reversed: number,
reversalObservable: boolean,
): number | null {
if (!reversalObservable) return null;
const decided = merged + closed;
if (decided <= 0) return null;
// `reversed` counts engine auto-actions regardless of a PR's CURRENT disposition, so a reopened
Expand Down Expand Up @@ -426,6 +462,10 @@ export async function getPublicStats(
error: 0,
reversed: 0,
};
// Resolved before byProject so every published accuracy figure -- per repo and global -- answers the same
// question about the same deployment, rather than one of them silently disagreeing with the others.
const totalReversals = [...reversedByProject.values()].reduce((sum, n) => sum + n, 0);
const reversalObservable = await loadReversalObservability(env, totalReversals);
const byProject = dispositions
.map((d) => {
const merged = d.merged ?? 0;
Expand All @@ -445,7 +485,7 @@ export async function getPublicStats(
reviewed,
merged,
closed,
accuracyPct: accuracyPct(merged, closed, reversed),
accuracyPct: accuracyPct(merged, closed, reversed, reversalObservable),
};
})
.filter((r) => r.reviewed > 0)
Expand Down Expand Up @@ -573,7 +613,7 @@ export async function getPublicStats(
// Option 1 of #7449: compute the global accuracy from the OWN-LEDGER merged/closed snapshot (not the
// fleet-folded totals.merged/closed), so its numerator (own-ledger reversed) and denominator are drawn
// from the same population. See the ownLedgerMerged/ownLedgerClosed snapshot above the Orb fold for why.
accuracyPct: accuracyPct(ownLedgerMerged, ownLedgerClosed, totals.reversed),
accuracyPct: accuracyPct(ownLedgerMerged, ownLedgerClosed, totals.reversed, reversalObservable),
minutesSaved,
},
weekly: { reviewed: w.reviewed ?? 0, merged: w.merged ?? 0 },
Expand Down
48 changes: 37 additions & 11 deletions src/services/public-accuracy-trend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// #2568 pattern for the sibling per-repo quality trend) can recompute any historical week correctly on every
// request -- no cron-miss gap risk, no second copy of the number to keep in sync, and the SAME formula as the
// live figure by construction, so the two can never silently diverge or read as inconsistent to a public viewer.
import { PUBLISHED_PR_KEYS, publicStatsProjects, safeAll } from "../review/public-stats";
import { loadReversalObservability, PUBLISHED_PR_KEYS, publicStatsProjects, safeAll } from "../review/public-stats";
import { isoWeekStart } from "./public-quality-metrics";

export const PUBLIC_ACCURACY_TREND_WEEKS = 8;
Expand All @@ -24,7 +24,13 @@ export type PublicAccuracyTrendWeek = {
accuracyPct: number | null;
};

type DayRow = { day: string; merged: number; closed: number; reversed: number };
/** `merged`/`closed` are the DISPLAYED volume (own ledger + registered Orb fleet). `ownMerged`/`ownClosed` are
* the own-ledger-only pairing for `reversed`, which is own-ledger-only by construction (the Orb aggregate has
* no reversal concept). Accuracy divides the own-ledger numbers ONLY -- #7449 fixed exactly this asymmetry for
* the lifetime figure in public-stats.ts and it was never carried across to this trend, so the denominator grew
* with every newly registered install while the numerator stayed own-ledger-scoped, trending every week toward
* 100% independent of real reversal behavior. */
type DayRow = { day: string; merged: number; closed: number; ownMerged: number; ownClosed: number; reversed: number };

const MS_PER_WEEK = 7 * 86_400_000;

Expand All @@ -33,20 +39,35 @@ function roundPct(value: number): number {
}

/** Same formula as public-stats.ts's accuracyPct, reused so the trend and the live number can never drift
* apart into two competing definitions of "accuracy". */
function publicBucketOf(bucket: { merged: number; closed: number; reversed: number }): Omit<PublicAccuracyTrendWeek, "weekStart"> {
* apart into two competing definitions of "accuracy". `reversalObservable` false means this deployment records
* no terminal auto-actions for a reversal to be attributed to, so the week's accuracy is unknown, not perfect --
* the volume columns still publish, since those ARE measured. */
function publicBucketOf(
bucket: { merged: number; closed: number; ownMerged: number; ownClosed: number; reversed: number },
reversalObservable: boolean,
): Omit<PublicAccuracyTrendWeek, "weekStart"> {
const decided = bucket.merged + bucket.closed;
if (decided < MIN_ACCURACY_TREND_SAMPLE) return { merged: null, closed: null, reversed: null, accuracyPct: null };
const reversalRate = Math.min(1, bucket.reversed / decided);
return { merged: bucket.merged, closed: bucket.closed, reversed: bucket.reversed, accuracyPct: roundPct(1 - reversalRate) };
const observed = { merged: bucket.merged, closed: bucket.closed, reversed: bucket.reversed };
if (!reversalObservable) return { ...observed, accuracyPct: null };
// Own-ledger-only denominator: `reversed` can only ever be attributed to own-ledger PRs (see DayRow).
const ownDecided = bucket.ownMerged + bucket.ownClosed;
if (ownDecided <= 0) return { ...observed, accuracyPct: null };
const reversalRate = Math.min(1, bucket.reversed / ownDecided);
return { ...observed, accuracyPct: roundPct(1 - reversalRate) };
}

/** Fold day-granularity rows into `weeks` trailing UTC-Monday buckets ending in the week containing `nowMs`.
* Pure -- mirrors buildPublicQualityTrend's own bucketing shape (public-quality-metrics.ts). */
export function buildPublicAccuracyTrend(dayRows: DayRow[], nowMs: number, weeks: number = PUBLIC_ACCURACY_TREND_WEEKS): PublicAccuracyTrendWeek[] {
export function buildPublicAccuracyTrend(
dayRows: DayRow[],
nowMs: number,
weeks: number = PUBLIC_ACCURACY_TREND_WEEKS,
reversalObservable = true,
): PublicAccuracyTrendWeek[] {
const currentStartMs = Date.parse(isoWeekStart(nowMs));
const oldestStartMs = currentStartMs - (weeks - 1) * MS_PER_WEEK;
const buckets = Array.from({ length: weeks }, () => ({ merged: 0, closed: 0, reversed: 0 }));
const buckets = Array.from({ length: weeks }, () => ({ merged: 0, closed: 0, ownMerged: 0, ownClosed: 0, reversed: 0 }));

for (const row of dayRows) {
const dayMs = Date.parse(`${row.day}T00:00:00.000Z`);
Expand All @@ -56,12 +77,14 @@ export function buildPublicAccuracyTrend(dayRows: DayRow[], nowMs: number, weeks
const bucket = buckets[weekOffset]!;
bucket.merged += row.merged;
bucket.closed += row.closed;
bucket.ownMerged += row.ownMerged;
bucket.ownClosed += row.ownClosed;
bucket.reversed += row.reversed;
}

return buckets.map((bucket, offset) => ({
weekStart: isoWeekStart(oldestStartMs + offset * MS_PER_WEEK),
...publicBucketOf(bucket),
...publicBucketOf(bucket, reversalObservable),
}));
}

Expand Down Expand Up @@ -169,19 +192,22 @@ export async function loadPublicAccuracyTrend(env: Env, nowMs: number = Date.now
const projects = publicStatsProjects(env);
const sinceIso = new Date(Date.parse(isoWeekStart(nowMs)) - (PUBLIC_ACCURACY_TREND_WEEKS - 1) * MS_PER_WEEK).toISOString();

const [ownLedger, reversals, orb] = await Promise.all([
const [ownLedger, reversals, orb, reversalObservable] = await Promise.all([
loadOwnLedgerDayRows(env, projects, sinceIso),
loadReversalDayRows(env, projects, sinceIso),
loadOrbDayRows(env, sinceIso),
loadReversalObservability(env),
]);

const days = new Set([...ownLedger.keys(), ...reversals.keys(), ...orb.keys()]);
const dayRows: DayRow[] = [...days].map((day) => ({
day,
merged: (ownLedger.get(day)?.merged ?? 0) + (orb.get(day)?.merged ?? 0),
closed: (ownLedger.get(day)?.closed ?? 0) + (orb.get(day)?.closed ?? 0),
ownMerged: ownLedger.get(day)?.merged ?? 0,
ownClosed: ownLedger.get(day)?.closed ?? 0,
reversed: reversals.get(day) ?? 0,
}));

return buildPublicAccuracyTrend(dayRows, nowMs);
return buildPublicAccuracyTrend(dayRows, nowMs, PUBLIC_ACCURACY_TREND_WEEKS, reversalObservable);
}
Loading
Loading