Skip to content

Commit 94fecf1

Browse files
feat(benchmark): leak-proof frozen-repo snapshot builder (#9259) (#9605)
* feat(benchmark): leak-proof frozen-repo snapshot builder (#9259) An agent must see EXACTLY the repository state a maintainer saw at commit T and nothing after it. A future-information leak inflates every score built on top, and it does so invisibly -- a leaked snapshot still produces perfectly well-formed numbers. That makes leak-proofing the deliverable rather than a property, which is why all filtering lives in a pure, exhaustively tested core instead of inline in the CLI. What a snapshot carries: open PRs and issues as of T, with labels filtered to those applied at or before T, plus gate decisions recorded at or before T. What it never carries: anything created after T (never admitted, so a downstream bug cannot reintroduce it), and the OUTCOME of any included unit -- an open PR's eventual merge/close is precisely what the agent is asked to predict, so carrying it would make the snapshot an answer key rather than a benchmark. The frozen type has no outcome field at all. The T boundary is inclusive and defined ONCE, in atOrBefore, so no field can quietly disagree with another. An unreadable timestamp is treated as not-visible: a date nobody can parse might be from after T, and the cost of excluding it is lost context rather than an inflated score. Two real defects were found and fixed while testing, both of which would have broken reproducibility silently: - checksumSnapshot spread its argument, so handing it a whole snapshot (the natural re-verification call) folded the existing checksum into its own preimage and returned a wrong digest with no error. The committed fields are now listed explicitly, making the function total over both shapes and making an added field a compile error at the commitment. - The decision sort tied on (workUnitId, decidedAt, action), so two decisions differing only in reasonCode ordered by input order and the checksum moved when the CLI read them the other way round. The chain now ends in a canonical-serialization comparison, total by construction. The CLI refuses to write a snapshot that fails an independent leak audit: an inflated benchmark is worse than no benchmark, because its numbers look fine. Closes #9259 * fix(benchmark): paginate the snapshot CLI's GitHub reads, and refuse a truncated snapshot Review caught both list reads stopping at page one. The consequence is worse than incompleteness: the snapshot's whole value is that two runs over the same (repo, T) produce the same checksum, and a truncated read makes that checksum a function of how many records the repo happened to have -- so the same T could yield two different "authoritative" snapshots. And it is silent, because the output is a perfectly well-formed snapshot that is simply missing history. - fetchAllPages reads to exhaustion, stopping at the first short page. - Both call sites use it: the issue/PR list, and per-issue label history. The second is the sharper of the two -- dropping early `labeled` events omits labels that were genuinely applied before T, which makes the snapshot WRONG rather than merely thinner. - Hitting the page bound is REPORTED, not silently accepted, and the CLI refuses to write a truncated snapshot -- the same posture as the leak audit, for the same reason: a snapshot nobody can reproduce is not one worth publishing. - A read error propagates instead of being swallowed into a short list that looks complete. The gap survived because the pagination lived inline in an untested CLI, so fetchAllPages is now an injectable seam with five tests covering multi-page reads, the short-page stop, the exactly-full-last-page extra read, empty/single-page sources, the bound, and error propagation. --------- Co-authored-by: loopover-orb[bot] <296761690+loopover-orb[bot]@users.noreply.github.com>
1 parent 3200abc commit 94fecf1

3 files changed

Lines changed: 878 additions & 0 deletions

File tree

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
// Frozen-repo snapshot builder, pure core (#9259, harness #9216, epic #8534).
2+
//
3+
// Scoring an arbitrary agent against realized history is only meaningful if the agent sees EXACTLY the
4+
// repository state a maintainer saw at commit T and NOTHING that happened after it. A future-information
5+
// leak silently inflates every score built on top — and it inflates them invisibly, because a leaked
6+
// snapshot still produces perfectly well-formed numbers. That makes leak-proofing the deliverable of this
7+
// module rather than a property of it, which is why the filtering is here, pure and exhaustively tested,
8+
// instead of inline in a CLI where it would be untestable.
9+
//
10+
// ── WHAT IS INCLUDED, AND WHAT IS DELIBERATELY NOT ───────────────────────────────────────────────────
11+
// INCLUDED, each filtered to its state as of `frozenAt`:
12+
// • openPullRequests — PRs created at or before T that were still open at T. Labels are filtered to
13+
// those applied at or before T; the body/title are the values as of T.
14+
// • openIssues — same rule.
15+
// • recentDecisions — gate decisions RECORDED at or before T. These are history the maintainer could
16+
// genuinely see, and they are what makes the task realistic rather than context-free.
17+
//
18+
// NOT INCLUDED, ever:
19+
// • Anything created after T. Not "filtered from the output" — never admitted, so a downstream bug
20+
// cannot reintroduce it.
21+
// • The OUTCOME of any included work unit. A snapshot carries the question, never the answer: an
22+
// open PR's eventual merge/close is exactly what the agent is being asked to predict, so a snapshot
23+
// that carried `state: "merged"` would not be a hard benchmark, it would be an answer key.
24+
// • Comments, labels, reviews, or status changes timestamped after T, even on an included record.
25+
//
26+
// ── THE BOUNDARY IS INCLUSIVE AT T ───────────────────────────────────────────────────────────────────
27+
// An event AT exactly `frozenAt` is included: T is the instant the maintainer is standing at, so what
28+
// happened at T is what they can see. Everything strictly after is the future. This is stated once, here,
29+
// and implemented once, in `atOrBefore`, so no field can quietly disagree with another.
30+
//
31+
// Pure: no IO, no clock, no randomness. `scripts/frozen-repo-snapshot.ts` is the thin CLI that does the
32+
// GitHub/DB reads and hands the raw records in. Checksum discipline mirrors `checksumCases` in
33+
// backtest-corpus-export-core.ts exactly (canonicalize with sorted keys, JSON-stringify, sha256).
34+
35+
import { createHash } from "node:crypto";
36+
37+
export const FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION = 1 as const;
38+
39+
/** A label with the instant it was APPLIED — the timestamp is what makes label filtering possible at all.
40+
* A raw GitHub label carries no application time; the CLI derives it from the issue-events timeline. */
41+
export type TimestampedLabel = { name: string; appliedAt: string };
42+
43+
/** One PR or issue as the CLI read it, with every time-varying field carrying its own timestamp. */
44+
export type RawWorkUnitRecord = {
45+
/** `owner/repo#123`. */
46+
workUnitId: string;
47+
number: number;
48+
kind: "pull_request" | "issue";
49+
title: string;
50+
body: string;
51+
authorLogin: string;
52+
createdAt: string;
53+
/** When it was closed/merged, if it ever was. Present in the RAW record and deliberately dropped from
54+
* the snapshot — see this module's header. Used only to decide open-at-T. */
55+
closedAt?: string | null | undefined;
56+
labels?: readonly TimestampedLabel[] | undefined;
57+
/** Changed file paths. A PR's file list is fixed at push time; the CLI supplies the list as of T. */
58+
changedPaths?: readonly string[] | undefined;
59+
};
60+
61+
/** A past gate decision the maintainer could see at T. */
62+
export type RawDecisionRecord = {
63+
workUnitId: string;
64+
action: string;
65+
reasonCode: string;
66+
decidedAt: string;
67+
};
68+
69+
/** One work unit as it appears IN the snapshot. Note what is absent: no `closedAt`, no state, no outcome
70+
* of any kind — the type itself refuses to carry the answer. */
71+
export type FrozenWorkUnit = {
72+
workUnitId: string;
73+
number: number;
74+
kind: "pull_request" | "issue";
75+
title: string;
76+
body: string;
77+
authorLogin: string;
78+
createdAt: string;
79+
/** Label NAMES only, sorted — the application timestamps did their job during filtering and would
80+
* otherwise be one more channel through which post-T information could travel. */
81+
labels: string[];
82+
changedPaths: string[];
83+
};
84+
85+
export type FrozenDecision = { workUnitId: string; action: string; reasonCode: string; decidedAt: string };
86+
87+
export type FrozenRepoSnapshot = {
88+
schemaVersion: typeof FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION;
89+
repoFullName: string;
90+
commitSha: string;
91+
frozenAt: string;
92+
openPullRequests: FrozenWorkUnit[];
93+
openIssues: FrozenWorkUnit[];
94+
recentDecisions: FrozenDecision[];
95+
/** sha256 over the canonicalized snapshot WITHOUT this field — so it commits to its own content and a
96+
* third party can confirm two runs scored the same task. */
97+
snapshotChecksum: string;
98+
};
99+
100+
/**
101+
* The single definition of "visible at T": at or before `frozenAt`, inclusive.
102+
*
103+
* An unparseable timestamp is NOT visible. That direction is deliberate and is the fail-safe one: a record
104+
* whose date cannot be read might be from after T, and admitting it would risk a leak, while excluding it
105+
* only costs a task some context. Every filter in this module routes through here, so the boundary cannot
106+
* drift between fields.
107+
*/
108+
export function atOrBefore(timestamp: string | null | undefined, frozenAt: string): boolean {
109+
if (!timestamp) return false;
110+
const at = Date.parse(timestamp);
111+
const cutoff = Date.parse(frozenAt);
112+
if (!Number.isFinite(at) || !Number.isFinite(cutoff)) return false;
113+
return at <= cutoff;
114+
}
115+
116+
/** Was this unit still OPEN at T? Closed strictly after T means it was open at T; closed at or before T
117+
* means it was already closed and is not part of the task. Never closed at all ⇒ open. */
118+
export function wasOpenAt(record: RawWorkUnitRecord, frozenAt: string): boolean {
119+
if (!atOrBefore(record.createdAt, frozenAt)) return false; // not created yet at T
120+
if (!record.closedAt) return true;
121+
// Closed at or before T ⇒ not open at T. An unparseable closedAt is treated as "still open", which is
122+
// the safe direction here: it keeps a question in the task rather than leaking a resolution.
123+
const closedAt = Date.parse(record.closedAt);
124+
if (!Number.isFinite(closedAt)) return true;
125+
return closedAt > Date.parse(frozenAt);
126+
}
127+
128+
/** Project one raw record onto its state at T. Labels applied after T are dropped; the outcome fields are
129+
* not carried at all. Sorting makes the output canonical, which is what lets the checksum be stable. */
130+
export function freezeWorkUnit(record: RawWorkUnitRecord, frozenAt: string): FrozenWorkUnit {
131+
const labels = (record.labels ?? [])
132+
.filter((label) => atOrBefore(label.appliedAt, frozenAt))
133+
.map((label) => label.name)
134+
.filter((name) => name.length > 0);
135+
return {
136+
workUnitId: record.workUnitId,
137+
number: record.number,
138+
kind: record.kind,
139+
title: record.title,
140+
body: record.body,
141+
authorLogin: record.authorLogin,
142+
createdAt: record.createdAt,
143+
labels: [...new Set(labels)].sort(),
144+
changedPaths: [...new Set(record.changedPaths ?? [])].sort(),
145+
};
146+
}
147+
148+
/** Total-order fallback: compare the canonicalized JSON. Reached only when every named key ties, and it
149+
* guarantees the sort is total no matter what fields the record type grows. */
150+
function compareCanonical(a: object, b: object): number {
151+
const left = JSON.stringify(canonicalize(a as Record<string, unknown>));
152+
const right = JSON.stringify(canonicalize(b as Record<string, unknown>));
153+
return left < right ? -1 : left > right ? 1 : 0;
154+
}
155+
156+
/** Sort key for every collection in a snapshot: the work-unit id, which is unique and stable. Sorting
157+
* rather than preserving input order is what makes two builds from differently-ordered reads identical. */
158+
function byWorkUnitId<T extends { workUnitId: string }>(a: T, b: T): number {
159+
return a.workUnitId < b.workUnitId ? -1 : a.workUnitId > b.workUnitId ? 1 : 0;
160+
}
161+
162+
/** Canonicalize with sorted keys — mirrors backtest-corpus-export-core.ts's `canonicalizeCase`. Two-armed
163+
* rather than the usual three: object keys are unique by definition, so an "equal keys" arm would be dead
164+
* code that no test could ever reach. */
165+
function canonicalize(value: Record<string, unknown>): Record<string, unknown> {
166+
return Object.fromEntries(Object.entries(value).sort(([a], [b]) => (a < b ? -1 : 1)));
167+
}
168+
169+
/**
170+
* Deterministic SHA-256 over the canonicalized snapshot body (every field except the checksum itself).
171+
*
172+
* The committed fields are listed EXPLICITLY rather than spread from the argument. Spreading made this a
173+
* foot-gun: handing it a whole `FrozenRepoSnapshot` (the natural thing to do when re-verifying) silently
174+
* folded the existing `snapshotChecksum` into the preimage and returned a different, wrong digest with no
175+
* error. Naming the fields makes the function total over both shapes and makes an added field a compile
176+
* error here — which is the right place to notice that the commitment needs updating.
177+
*/
178+
export function checksumSnapshot(snapshot: Omit<FrozenRepoSnapshot, "snapshotChecksum">): string {
179+
const canonical = canonicalize({
180+
schemaVersion: snapshot.schemaVersion,
181+
repoFullName: snapshot.repoFullName,
182+
commitSha: snapshot.commitSha,
183+
frozenAt: snapshot.frozenAt,
184+
openPullRequests: snapshot.openPullRequests.map((unit) => canonicalize(unit as unknown as Record<string, unknown>)),
185+
openIssues: snapshot.openIssues.map((unit) => canonicalize(unit as unknown as Record<string, unknown>)),
186+
recentDecisions: snapshot.recentDecisions.map((decision) => canonicalize(decision as unknown as Record<string, unknown>)),
187+
});
188+
return createHash("sha256").update(JSON.stringify(canonical)).digest("hex");
189+
}
190+
191+
/**
192+
* Build a leak-proof snapshot of a repo at commit T.
193+
*
194+
* Every collection is filtered through {@link atOrBefore} and sorted, so the result is a pure function of
195+
* (records, frozenAt) — never of when the build ran, nor of the order the CLI happened to read records in.
196+
* That is the property the benchmark's reproducibility rests on, and it is asserted directly in the tests.
197+
*/
198+
export function buildFrozenRepoSnapshot(input: {
199+
repoFullName: string;
200+
commitSha: string;
201+
frozenAt: string;
202+
workUnits: readonly RawWorkUnitRecord[];
203+
decisions?: readonly RawDecisionRecord[] | undefined;
204+
}): FrozenRepoSnapshot {
205+
const openAtT = input.workUnits.filter((record) => wasOpenAt(record, input.frozenAt));
206+
const openPullRequests = openAtT
207+
.filter((record) => record.kind === "pull_request")
208+
.map((record) => freezeWorkUnit(record, input.frozenAt))
209+
.sort(byWorkUnitId);
210+
const openIssues = openAtT
211+
.filter((record) => record.kind === "issue")
212+
.map((record) => freezeWorkUnit(record, input.frozenAt))
213+
.sort(byWorkUnitId);
214+
const recentDecisions = (input.decisions ?? [])
215+
.filter((decision) => atOrBefore(decision.decidedAt, input.frozenAt))
216+
.map((decision) => ({
217+
workUnitId: decision.workUnitId,
218+
action: decision.action,
219+
reasonCode: decision.reasonCode,
220+
decidedAt: decision.decidedAt,
221+
}))
222+
// Decisions can repeat per work unit, so the id alone is not a total order. The tie-break chain must
223+
// cover EVERY field, not just the obvious ones: two decisions differing only in `reasonCode` compared
224+
// equal under an earlier version of this sort, so their relative order followed input order and the
225+
// snapshot checksum moved when the CLI happened to read them the other way round -- which is exactly
226+
// the reproducibility property this module exists to guarantee. Comparing the canonical serialization
227+
// last makes the order total by construction, so adding a field to FrozenDecision cannot silently
228+
// reintroduce the same hole.
229+
.sort(
230+
(a, b) =>
231+
byWorkUnitId(a, b) ||
232+
(a.decidedAt < b.decidedAt ? -1 : a.decidedAt > b.decidedAt ? 1 : 0) ||
233+
(a.action < b.action ? -1 : a.action > b.action ? 1 : 0) ||
234+
compareCanonical(a, b),
235+
);
236+
237+
const body: Omit<FrozenRepoSnapshot, "snapshotChecksum"> = {
238+
schemaVersion: FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION,
239+
repoFullName: input.repoFullName,
240+
commitSha: input.commitSha,
241+
frozenAt: input.frozenAt,
242+
openPullRequests,
243+
openIssues,
244+
recentDecisions,
245+
};
246+
return { ...body, snapshotChecksum: checksumSnapshot(body) };
247+
}
248+
249+
/** Recompute a snapshot's checksum and compare — the exact check a third party runs to confirm two runs
250+
* scored the same task, exported so our tests exercise the SAME path rather than a parallel one. */
251+
export function verifySnapshotChecksum(snapshot: FrozenRepoSnapshot): boolean {
252+
const { snapshotChecksum, ...body } = snapshot;
253+
return checksumSnapshot(body) === snapshotChecksum;
254+
}
255+
256+
/**
257+
* Audit a built snapshot for future information — a belt-and-braces check over the builder's own output.
258+
*
259+
* The builder already excludes post-T data by construction; this re-derives the property independently, so
260+
* a future refactor that breaks the filtering fails loudly here instead of silently inflating scores. It
261+
* returns the offending paths rather than a bare boolean, because "which field leaked" is the only useful
262+
* form of that answer.
263+
*/
264+
export function auditSnapshotForLeaks(snapshot: FrozenRepoSnapshot): string[] {
265+
const leaks: string[] = [];
266+
const check = (collection: "openPullRequests" | "openIssues") => {
267+
for (const unit of snapshot[collection]) {
268+
if (!atOrBefore(unit.createdAt, snapshot.frozenAt)) leaks.push(`${collection}/${unit.workUnitId}: createdAt is after frozenAt`);
269+
// The snapshot type carries no outcome fields, but a hand-built or deserialized object could.
270+
for (const forbidden of ["closedAt", "mergedAt", "state", "merged"]) {
271+
if (forbidden in (unit as unknown as Record<string, unknown>)) leaks.push(`${collection}/${unit.workUnitId}: carries outcome field "${forbidden}"`);
272+
}
273+
}
274+
};
275+
check("openPullRequests");
276+
check("openIssues");
277+
for (const decision of snapshot.recentDecisions) {
278+
if (!atOrBefore(decision.decidedAt, snapshot.frozenAt)) leaks.push(`recentDecisions/${decision.workUnitId}: decidedAt is after frozenAt`);
279+
}
280+
return leaks;
281+
}

0 commit comments

Comments
 (0)