Skip to content

Commit f4c488a

Browse files
authored
Merge branch 'master' into claude/pr-456-20260614-0714
2 parents 6cbb380 + e7ee836 commit f4c488a

3 files changed

Lines changed: 93 additions & 4 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { compareBenchmarkRecency } from './json-provider.js';
4+
5+
/**
6+
* `compareBenchmarkRecency` is the shared dedup ordering used by both the SQL
7+
* date-filtered query (ORDER BY br.date DESC, wr.run_started_at DESC NULLS LAST)
8+
* and the JSON provider's getLatestBenchmarks. A negative result means `a` sorts
9+
* before `b`, so `a` is the more-recent record kept by DISTINCT ON.
10+
*
11+
* Regression guard for the same-day multi-run bug: when a config is swept more
12+
* than once on the same calendar day, the later sweep (greater run_started_at)
13+
* must win — otherwise the earlier run's points shadow the re-sweep on the chart.
14+
*/
15+
describe('compareBenchmarkRecency', () => {
16+
const later = '2026-06-14T06:43:25Z';
17+
const earlier = '2026-06-14T04:08:16Z';
18+
19+
it('orders a more recent calendar day first regardless of run_started_at', () => {
20+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-13', earlier, later)).toBeLessThan(0);
21+
expect(compareBenchmarkRecency('2026-06-13', '2026-06-14', later, earlier)).toBeGreaterThan(0);
22+
});
23+
24+
it('tiebreaks same-day sweeps by run_started_at (latest sweep wins)', () => {
25+
// a = later sweep → a should sort first (negative).
26+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-14', later, earlier)).toBeLessThan(0);
27+
// a = earlier sweep → a should sort after (positive).
28+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-14', earlier, later)).toBeGreaterThan(0);
29+
});
30+
31+
it('sorts a null run_started_at last on a same-day tie', () => {
32+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-14', null, earlier)).toBeGreaterThan(0);
33+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-14', earlier, null)).toBeLessThan(0);
34+
});
35+
36+
it('treats equal date and equal run_started_at as a tie', () => {
37+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-14', later, later)).toBe(0);
38+
expect(compareBenchmarkRecency('2026-06-14', '2026-06-14', null, null)).toBe(0);
39+
});
40+
41+
it('keeps the latest sweep first when sorting a same-day candidate list', () => {
42+
interface Row {
43+
run: string;
44+
started: string | null;
45+
}
46+
const rows: Row[] = [
47+
{ run: 'old', started: earlier },
48+
{ run: 'new', started: later },
49+
{ run: 'unknown', started: null },
50+
];
51+
rows.sort((a, b) => compareBenchmarkRecency('2026-06-14', '2026-06-14', a.started, b.started));
52+
expect(rows.map((r) => r.run)).toEqual(['new', 'old', 'unknown']);
53+
});
54+
});

packages/db/src/json-provider.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,29 @@ const STRIP_HISTORY_KEYS = new Set([
328328
'mean_itl',
329329
]);
330330

331+
/**
332+
* Comparator for DISTINCT ON (config, conc, isl, osl) selection: latest calendar
333+
* day first, then — for sweeps on the same day — the latest workflow run first by
334+
* `run_started_at` (NULLS LAST). Mirrors the SQL date-filtered query and the
335+
* `latest_benchmarks` view (migration 003): a calendar day alone ties two same-day
336+
* sweeps, so without this an older run's points can shadow a same-day re-sweep.
337+
* `run_started_at` is an ISO-8601 string, so localeCompare orders it chronologically.
338+
* Exported so the same-day tiebreak is unit-tested in parity with the SQL.
339+
*/
340+
export function compareBenchmarkRecency(
341+
aDate: string,
342+
bDate: string,
343+
aStarted: string | null,
344+
bStarted: string | null,
345+
): number {
346+
const dateCmp = bDate.localeCompare(aDate);
347+
if (dateCmp !== 0) return dateCmp;
348+
if (aStarted === bStarted) return 0;
349+
if (aStarted === null) return 1;
350+
if (bStarted === null) return -1;
351+
return bStarted.localeCompare(aStarted);
352+
}
353+
331354
export function getLatestBenchmarks(
332355
modelKey: string | string[],
333356
date?: string,
@@ -350,10 +373,17 @@ export function getLatestBenchmarks(
350373
return true;
351374
});
352375

353-
// DISTINCT ON (config_id, conc, isl, osl) — keep the one with the latest date
376+
// DISTINCT ON (config_id, conc, isl, osl) — keep the one with the latest date,
377+
// tiebreaking same-day runs by run_started_at so the latest sweep wins.
354378
const seen = new Map<string, RawBenchmarkResult>();
355-
// Sort by date DESC so first-seen wins
356-
candidates.sort((a, b) => toDateString(b.date).localeCompare(toDateString(a.date)));
379+
candidates.sort((a, b) =>
380+
compareBenchmarkRecency(
381+
toDateString(a.date),
382+
toDateString(b.date),
383+
s.latestRunsById.get(a.workflow_run_id)?.run_started_at ?? null,
384+
s.latestRunsById.get(b.workflow_run_id)?.run_started_at ?? null,
385+
),
386+
);
357387
for (const br of candidates) {
358388
const key = `${br.config_id}:${br.conc}:${br.isl}:${br.osl}`;
359389
if (!seen.has(key)) seen.set(key, br);

packages/db/src/queries/benchmarks.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ export async function getLatestBenchmarks(
6565
// Date-filtered: use base table with DISTINCT ON (the view only has the absolute latest)
6666
// exact=true: only return data from this exact date (for GPU comparison)
6767
// exact=false (default): return latest data as of this date (for main chart)
68+
// Same-day tiebreak by wr.run_started_at (latest sweep wins), mirroring the
69+
// latest_benchmarks view (migration 003). br.date is a calendar day, so two
70+
// sweeps on the same day tie on date alone and Postgres would otherwise pick
71+
// an arbitrary one — leaving an older run's points shadowing a same-day re-sweep.
6872
const dateFilter = exact ? sql`br.date = ${date}::date` : sql`br.date <= ${date}::date`;
6973
const rows = await sql`
7074
SELECT DISTINCT ON (br.config_id, br.conc, br.isl, br.osl)
@@ -99,7 +103,8 @@ export async function getLatestBenchmarks(
99103
WHERE c.model = ANY(${modelKeys})
100104
AND br.error IS NULL
101105
AND ${dateFilter}
102-
ORDER BY br.config_id, br.conc, br.isl, br.osl, br.date DESC
106+
ORDER BY br.config_id, br.conc, br.isl, br.osl,
107+
br.date DESC, wr.run_started_at DESC NULLS LAST
103108
`;
104109
return rows as unknown as BenchmarkRow[];
105110
}

0 commit comments

Comments
 (0)