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
19 changes: 17 additions & 2 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,11 @@ export async function upsertPullRequestFromGitHub(
headShaObservedAt,
lastSeenOpenAt,
payloadJson: jsonString(payload),
// GitHub's own PR creation time (see PullRequestRecord.createdAt's doc comment, src/types.ts) --
// set ONLY here, on first insert, and deliberately absent from onConflictDoUpdate's `set` below so
// a resync never overwrites it. `?? undefined` falls through to the column's own $defaultFn when a
// sparse payload omits created_at, matching every other optional GitHub-sourced field's convention.
createdAt: pr.created_at ?? undefined,
updatedAt: syncedAt,
})
.onConflictDoUpdate({
Expand Down Expand Up @@ -4254,15 +4259,25 @@ export async function markUnseenOpenPullRequestsClosed(env: Env, fullName: strin
return Number(result.meta.changes ?? 0);
}

// Ordered by DESCENDING PR number so a repo with >500 PRs keeps its MOST RECENT ones, not an arbitrary/
// unordered slice a plain LIMIT would otherwise return (Postgres gives no ordering guarantee without
// ORDER BY -- confirmed live: an unordered 500-row cap on a 2930-row repo produced a skewed, non-
// representative sample that inverted src/services/outcome-calibration.ts's slop-band merge-rate check,
// which reads this exact function, and fired a false "score not discriminating" ops_anomaly). Every other
// caller (MCP tools, gate-precision, quality metrics, recap) benefits the same way: recent PRs are the
// relevant population for almost every one of them, an arbitrary old slice never was.
export async function listPullRequests(env: Env, fullName: string): Promise<PullRequestRecord[]> {
const db = getDb(env.DB);
const rows = await db.select().from(pullRequests).where(eq(pullRequests.repoFullName, fullName)).limit(500);
const rows = await db.select().from(pullRequests).where(eq(pullRequests.repoFullName, fullName)).orderBy(desc(pullRequests.number)).limit(500);
return rows.map(toPullRequestRecordFromRow);
}

// Same ordering rationale as listPullRequests above, but cross-repo -- PR number resets per repo, so
// createdAt (an ISO 8601 UTC string, sortable lexicographically) is the only field that's globally
// comparable for "most recent" across every repo at once.
export async function listAllPullRequests(env: Env): Promise<PullRequestRecord[]> {
const db = getDb(env.DB);
const rows = await db.select().from(pullRequests).limit(2000);
const rows = await db.select().from(pullRequests).orderBy(desc(pullRequests.createdAt)).limit(2000);
return rows.map(toPullRequestRecordFromRow);
}

Expand Down
49 changes: 49 additions & 0 deletions test/unit/db-parsers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getLatestScoringModelSnapshot,
getFreshOfficialMinerDetection,
getPullRequest,
listAllPullRequests,
listPullRequests,
listPullRequestDetailSyncStates,
listRepoSyncSegments,
Expand Down Expand Up @@ -1336,3 +1337,51 @@ describe("database row parser hardening", () => {
expect(JSON.parse(raw?.snapshot_json ?? "{}")).toMatchObject({ githubId: "", githubUsername: "", issueLabels: [] });
});
});

// #ops-anomaly-calibration-sample-order: both list functions cap their row count, and a LIMIT with no ORDER BY
// gives no ordering guarantee -- confirmed live on a 2930-row repo, where the resulting arbitrary 500-row slice
// inverted src/services/outcome-calibration.ts's slop-band merge-rate comparison (which reads listPullRequests)
// and fired a false ops_anomaly "score not discriminating" alert, even though the score discriminates correctly
// (monotonically decreasing merge rate by rising severity) over the true full population. Ordering by recency
// fixes this at the source, for every caller, not just calibration.
describe("listPullRequests / listAllPullRequests ordering (#ops-anomaly-calibration-sample-order)", () => {
it("listPullRequests returns a repo's PRs ordered by DESCENDING number, regardless of insert order", async () => {
const env = createTestEnv();
for (const number of [3, 1, 4, 2]) {
await upsertPullRequestFromGitHub(env, "owner/repo", {
number,
title: `PR #${number}`,
state: "open",
user: { login: "contributor1" },
labels: [],
body: null,
});
}

const numbers = (await listPullRequests(env, "owner/repo")).map((pr) => pr.number);
expect(numbers).toEqual([4, 3, 2, 1]);
});

it("listAllPullRequests returns PRs ordered by DESCENDING createdAt across repos, regardless of insert order", async () => {
const env = createTestEnv();
const seeds: Array<{ repoFullName: string; number: number; createdAt: string }> = [
{ repoFullName: "owner/repo-a", number: 1, createdAt: "2026-01-01T00:00:00.000Z" },
{ repoFullName: "owner/repo-b", number: 1, createdAt: "2026-03-01T00:00:00.000Z" },
{ repoFullName: "owner/repo-a", number: 2, createdAt: "2026-02-01T00:00:00.000Z" },
];
for (const seed of seeds) {
await upsertPullRequestFromGitHub(env, seed.repoFullName, {
number: seed.number,
title: `${seed.repoFullName}#${seed.number}`,
state: "open",
user: { login: "contributor1" },
labels: [],
body: null,
created_at: seed.createdAt,
});
}

const ordered = (await listAllPullRequests(env)).map((pr) => `${pr.repoFullName}#${pr.number}`);
expect(ordered).toEqual(["owner/repo-b#1", "owner/repo-a#2", "owner/repo-a#1"]);
});
});