Skip to content

Commit 5b9e9e1

Browse files
authored
fix(orb): stop double-counting PRs already tracked by the own-ledger (#5257)
getOrbGlobalStats folded in every registered Orb installation's outcomes unconditionally, re-counting PRs the frozen own-ledger (pull_requests + audit_events, frozen since the self-host cutover) already counted. Quantified: 243 PRs (173 merged + 70 closed, 96% in one repo), inflating the public homepage's "PRs reviewed" and "Decision accuracy" tiles. Adds a NOT EXISTS anti-join so orb_pr_outcomes rows whose (repo, pr_number) already has a github_app.pr_public_surface_published audit event are excluded — the two sums are now over disjoint PR sets. No data loss: the own-ledger side is untouched, and no backfill is needed since the two sources' actual PR-number ranges barely overlap outside the double-counted set.
1 parent 45a3741 commit 5b9e9e1

4 files changed

Lines changed: 46 additions & 12 deletions

File tree

src/orb/outcomes.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,16 @@ export interface OrbGlobalStats {
4141
* The public global aggregate: merged / closed / total terminal PR outcomes across REGISTERED installations
4242
* only (registered = 1) — an install that hasn't been opted in never contributes to the public counter. SUM over
4343
* no matching rows is NULL, so each total is nullish-guarded to 0 (fail-safe on an empty/cold table).
44+
*
45+
* The NOT EXISTS anti-join skips any (repo, pr_number) that already has a `github_app.pr_public_surface_published`
46+
* audit event — i.e. a PR the own-ledger disposition query in public-stats.ts already counted. Without it, a PR
47+
* reviewed before the self-host cutover (own-ledger) that also has a terminal outcome recorded here (Orb) gets
48+
* counted twice. Quantified 2026-07-12: 243 PRs (173 merged + 70 closed), 96% in one repo, inflating the public
49+
* "PRs reviewed" counter. That event_type's target_key only ever references the own-ledger's own repos, so this
50+
* is a no-op for every other registered installation's outcomes.
4451
*/
4552
export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: string } = {}): Promise<OrbGlobalStats> {
46-
// excludeAccount de-dups an account already counted by another source — the homepage counts JSONbored's own
47-
// repos via cloud review_audit, so it excludes that account here to avoid double-counting. "" = include all.
53+
// excludeAccount de-dups an account already counted by another source. "" = include all.
4854
const exclude = (opts.excludeAccount ?? "").toLowerCase();
4955
const row = await env.DB.prepare(
5056
`SELECT
@@ -53,7 +59,12 @@ export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: strin
5359
COUNT(*) AS total
5460
FROM orb_pr_outcomes o
5561
JOIN orb_github_installations i ON i.installation_id = o.installation_id AND i.registered = 1
56-
WHERE ? = '' OR LOWER(COALESCE(i.account_login, '')) <> ?`,
62+
WHERE (? = '' OR LOWER(COALESCE(i.account_login, '')) <> ?)
63+
AND NOT EXISTS (
64+
SELECT 1 FROM audit_events ae
65+
WHERE ae.event_type = 'github_app.pr_public_surface_published'
66+
AND LOWER(ae.target_key) = LOWER(o.repository_full_name) || '#' || o.pr_number
67+
)`,
5768
)
5869
.bind(exclude, exclude)
5970
.first<{ merged: number | null; closed: number | null; total: number | null }>();

src/review/public-stats.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,15 @@
2525
// GLOBAL: the homepage total folds in every REGISTERED Orb installation's outcomes (getOrbGlobalStats) on top of
2626
// the own-ledger side, so the counter reflects the whole fleet, not just gittensory's own repos. The own-ledger
2727
// side (audit_events) is a FROZEN snapshot as of the self-host cutover -- it stops growing the day each repo's
28-
// live processing moved off this worker -- while orb_pr_outcomes keeps growing in realtime for any repo with the
29-
// central Orb App installed (including JSONbored's own, which still runs the Orb App for telemetry alongside its
30-
// self-hosted review engine). The two sources are NOT mutually exclusive by account: JSONbored appears in both,
31-
// but only overlaps for the few-day window before the cutover froze the own-ledger side, so no excludeAccount is
32-
// applied here -- a small, bounded double-count for that historical window is preferable to silently dropping
33-
// all of JSONbored's live post-cutover Orb data (which excluding "jsonbored" used to do, since every REGISTERED
34-
// installation currently happens to be a JSONbored account). See getOrbGlobalStats for the general-purpose
35-
// excludeAccount dedup this call deliberately does not use.
28+
// live processing moved off this worker, and can never grow again now that the old App has been fully deleted --
29+
// while orb_pr_outcomes keeps growing in realtime for any repo with the central Orb App installed (including
30+
// JSONbored's own, which still runs the Orb App for telemetry alongside its self-hosted review engine). The two
31+
// sources overlap by (repo, pr_number), not just by account: earlier reasoning here called that overlap "a small,
32+
// bounded double-count," but it was measured directly (2026-07-12) at 243 PRs (173 merged + 70 closed, 96% in one
33+
// repo) -- large enough to move accuracyPct and mislead visitors, not a rounding error. getOrbGlobalStats now
34+
// excludes exactly those overlapping (repo, pr_number) pairs via a NOT EXISTS anti-join against the same
35+
// `github_app.pr_public_surface_published` audit events this file's own disposition query reads, so the two sums
36+
// are over disjoint PR sets and can be added directly.
3637
import { getOrbGlobalStats } from "../orb/outcomes";
3738

3839
/** FALLBACK estimate of maintainer review/triage time saved per reviewed PR, used ONLY when the real per-PR

test/integration/orb-outcomes.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,19 @@ describe("getOrbGlobalStats", () => {
9696
expect(await getOrbGlobalStats(e, { excludeAccount: "jsonbored" })).toEqual({ merged: 0, closed: 1, total: 1 }); // JSONbored dropped
9797
expect(await getOrbGlobalStats(e)).toEqual({ merged: 1, closed: 1, total: 2 }); // no exclude → both
9898
});
99+
100+
it("excludes a PR already counted by the own-ledger (published-surface audit event), but still counts an unrelated one", async () => {
101+
const e = createTestEnv();
102+
const db = e.DB as unknown as TestD1Database;
103+
await registerInstall(e, 100, 1);
104+
// acme/dup#1 was already published (and therefore counted) by the own-ledger disposition query.
105+
await db
106+
.prepare(
107+
"INSERT INTO audit_events (id, event_type, target_key, outcome, created_at) VALUES ('evt-1', 'github_app.pr_public_surface_published', 'acme/dup#1', 'success', '2026-06-24T00:00:00Z')",
108+
)
109+
.run();
110+
await recordOrbPrOutcome(e, "pull_request", closedPr("acme/dup", 1, "2026-06-24T00:00:00Z", 100)); // merged, already own-ledger-counted → must be excluded
111+
await recordOrbPrOutcome(e, "pull_request", closedPr("acme/new", 2, null, 100)); // closed, no own-ledger counterpart → must still count
112+
expect(await getOrbGlobalStats(e)).toEqual({ merged: 0, closed: 1, total: 1 });
113+
});
99114
});

test/unit/public-stats.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,18 @@ function isWeekly(sql: string): boolean {
3434
function isEffort(sql: string): boolean {
3535
return sql.includes("reviewEffortMinutes");
3636
}
37+
// getOrbGlobalStats's own-ledger anti-join also references `github_app.pr_public_surface_published` (to skip
38+
// PRs the disposition query already counted) — exclude it here the same way isWeekly/isEffort already are, or
39+
// every stub that doesn't special-case `orb_pr_outcomes` would wrongly route the orb read through here too.
40+
function isOrbGlobal(sql: string): boolean {
41+
return sql.includes("orb_pr_outcomes");
42+
}
3743
function isDispositions(sql: string): boolean {
3844
return (
3945
sql.includes("github_app.pr_public_surface_published") &&
4046
!isWeekly(sql) &&
41-
!isEffort(sql)
47+
!isEffort(sql) &&
48+
!isOrbGlobal(sql)
4249
);
4350
}
4451
// The reversal read is the only one that inspects engine auto-actions (close/merge) against pull_requests state.

0 commit comments

Comments
 (0)