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
17 changes: 14 additions & 3 deletions src/orb/outcomes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,16 @@ export interface OrbGlobalStats {
* The public global aggregate: merged / closed / total terminal PR outcomes across REGISTERED installations
* only (registered = 1) — an install that hasn't been opted in never contributes to the public counter. SUM over
* no matching rows is NULL, so each total is nullish-guarded to 0 (fail-safe on an empty/cold table).
*
* The NOT EXISTS anti-join skips any (repo, pr_number) that already has a `github_app.pr_public_surface_published`
* audit event — i.e. a PR the own-ledger disposition query in public-stats.ts already counted. Without it, a PR
* reviewed before the self-host cutover (own-ledger) that also has a terminal outcome recorded here (Orb) gets
* counted twice. Quantified 2026-07-12: 243 PRs (173 merged + 70 closed), 96% in one repo, inflating the public
* "PRs reviewed" counter. That event_type's target_key only ever references the own-ledger's own repos, so this
* is a no-op for every other registered installation's outcomes.
*/
export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: string } = {}): Promise<OrbGlobalStats> {
// excludeAccount de-dups an account already counted by another source — the homepage counts JSONbored's own
// repos via cloud review_audit, so it excludes that account here to avoid double-counting. "" = include all.
// excludeAccount de-dups an account already counted by another source. "" = include all.
const exclude = (opts.excludeAccount ?? "").toLowerCase();
const row = await env.DB.prepare(
`SELECT
Expand All @@ -53,7 +59,12 @@ export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: strin
COUNT(*) AS total
FROM orb_pr_outcomes o
JOIN orb_github_installations i ON i.installation_id = o.installation_id AND i.registered = 1
WHERE ? = '' OR LOWER(COALESCE(i.account_login, '')) <> ?`,
WHERE (? = '' OR LOWER(COALESCE(i.account_login, '')) <> ?)
AND NOT EXISTS (
SELECT 1 FROM audit_events ae
WHERE ae.event_type = 'github_app.pr_public_surface_published'
AND LOWER(ae.target_key) = LOWER(o.repository_full_name) || '#' || o.pr_number
)`,
)
.bind(exclude, exclude)
.first<{ merged: number | null; closed: number | null; total: number | null }>();
Expand Down
17 changes: 9 additions & 8 deletions src/review/public-stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@
// GLOBAL: the homepage total folds in every REGISTERED Orb installation's outcomes (getOrbGlobalStats) on top of
// the own-ledger side, so the counter reflects the whole fleet, not just gittensory's own repos. The own-ledger
// side (audit_events) is a FROZEN snapshot as of the self-host cutover -- it stops growing the day each repo's
// live processing moved off this worker -- while orb_pr_outcomes keeps growing in realtime for any repo with the
// central Orb App installed (including JSONbored's own, which still runs the Orb App for telemetry alongside its
// self-hosted review engine). The two sources are NOT mutually exclusive by account: JSONbored appears in both,
// but only overlaps for the few-day window before the cutover froze the own-ledger side, so no excludeAccount is
// applied here -- a small, bounded double-count for that historical window is preferable to silently dropping
// all of JSONbored's live post-cutover Orb data (which excluding "jsonbored" used to do, since every REGISTERED
// installation currently happens to be a JSONbored account). See getOrbGlobalStats for the general-purpose
// excludeAccount dedup this call deliberately does not use.
// live processing moved off this worker, and can never grow again now that the old App has been fully deleted --
// while orb_pr_outcomes keeps growing in realtime for any repo with the central Orb App installed (including
// JSONbored's own, which still runs the Orb App for telemetry alongside its self-hosted review engine). The two
// sources overlap by (repo, pr_number), not just by account: earlier reasoning here called that overlap "a small,
// bounded double-count," but it was measured directly (2026-07-12) at 243 PRs (173 merged + 70 closed, 96% in one
// repo) -- large enough to move accuracyPct and mislead visitors, not a rounding error. getOrbGlobalStats now
// excludes exactly those overlapping (repo, pr_number) pairs via a NOT EXISTS anti-join against the same
// `github_app.pr_public_surface_published` audit events this file's own disposition query reads, so the two sums
// are over disjoint PR sets and can be added directly.
import { getOrbGlobalStats } from "../orb/outcomes";

/** FALLBACK estimate of maintainer review/triage time saved per reviewed PR, used ONLY when the real per-PR
Expand Down
15 changes: 15 additions & 0 deletions test/integration/orb-outcomes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,19 @@ describe("getOrbGlobalStats", () => {
expect(await getOrbGlobalStats(e, { excludeAccount: "jsonbored" })).toEqual({ merged: 0, closed: 1, total: 1 }); // JSONbored dropped
expect(await getOrbGlobalStats(e)).toEqual({ merged: 1, closed: 1, total: 2 }); // no exclude → both
});

it("excludes a PR already counted by the own-ledger (published-surface audit event), but still counts an unrelated one", async () => {
const e = createTestEnv();
const db = e.DB as unknown as TestD1Database;
await registerInstall(e, 100, 1);
// acme/dup#1 was already published (and therefore counted) by the own-ledger disposition query.
await db
.prepare(
"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')",
)
.run();
await recordOrbPrOutcome(e, "pull_request", closedPr("acme/dup", 1, "2026-06-24T00:00:00Z", 100)); // merged, already own-ledger-counted → must be excluded
await recordOrbPrOutcome(e, "pull_request", closedPr("acme/new", 2, null, 100)); // closed, no own-ledger counterpart → must still count
expect(await getOrbGlobalStats(e)).toEqual({ merged: 0, closed: 1, total: 1 });
});
});
9 changes: 8 additions & 1 deletion test/unit/public-stats.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,18 @@ function isWeekly(sql: string): boolean {
function isEffort(sql: string): boolean {
return sql.includes("reviewEffortMinutes");
}
// getOrbGlobalStats's own-ledger anti-join also references `github_app.pr_public_surface_published` (to skip
// PRs the disposition query already counted) — exclude it here the same way isWeekly/isEffort already are, or
// every stub that doesn't special-case `orb_pr_outcomes` would wrongly route the orb read through here too.
function isOrbGlobal(sql: string): boolean {
return sql.includes("orb_pr_outcomes");
}
function isDispositions(sql: string): boolean {
return (
sql.includes("github_app.pr_public_surface_published") &&
!isWeekly(sql) &&
!isEffort(sql)
!isEffort(sql) &&
!isOrbGlobal(sql)
);
}
// The reversal read is the only one that inspects engine auto-actions (close/merge) against pull_requests state.
Expand Down