From 8d31880492f56c2c8a21f36f79c5915486617d57 Mon Sep 17 00:00:00 2001 From: Alexander Karan <47707063+AlexanderKaran@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:49:08 +0800 Subject: [PATCH] Added Variance Check --- .github/workflows/benchmark-consistency.yml | 40 ++++++ packages/results/package.json | 1 + packages/results/src/comparison.ts | 125 ++++++++++++++++++ packages/results/src/measurements.ts | 2 + packages/results/src/schemas.ts | 9 ++ packages/stats-generator/package.json | 1 + .../src/benchmark-consistency.ts | 39 ++++++ .../src/check-benchmark-consistency.ts | 88 ++++++++++++ packages/stats-generator/src/save-ci-stats.ts | 27 +++- 9 files changed, 326 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/benchmark-consistency.yml create mode 100644 packages/results/src/comparison.ts create mode 100644 packages/stats-generator/src/benchmark-consistency.ts create mode 100644 packages/stats-generator/src/check-benchmark-consistency.ts diff --git a/.github/workflows/benchmark-consistency.yml b/.github/workflows/benchmark-consistency.yml new file mode 100644 index 00000000..b1f018d7 --- /dev/null +++ b/.github/workflows/benchmark-consistency.yml @@ -0,0 +1,40 @@ +name: Benchmark consistency + +on: + pull_request: + branches: [main] + paths: + - 'results/**' + +permissions: + contents: read + +jobs: + check: + name: Check summary changes + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + + - name: Setup pnpm + uses: pnpm/action-setup@8912a9102ac27614460f54aedde9e1e7f9aec20d # v6.0.5 + + - name: Setup Node.js + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: '24' + cache: 'pnpm' + + - name: Install checker dependencies + run: pnpm install --frozen-lockfile --filter @framework-tracker/stats-generator... + + - name: Check benchmark consistency + env: + BASE_REF: ${{ github.event.pull_request.base.sha }} + run: | + mkdir -p "$RUNNER_TEMP/benchmark-baseline" + git archive "$BASE_REF" results | tar -x -C "$RUNNER_TEMP/benchmark-baseline" + pnpm --filter @framework-tracker/stats-generator check:benchmark-consistency "$RUNNER_TEMP/benchmark-baseline/results" diff --git a/packages/results/package.json b/packages/results/package.json index 21a03bd1..1cb091fc 100644 --- a/packages/results/package.json +++ b/packages/results/package.json @@ -8,6 +8,7 @@ "./selection": "./src/selection.ts", "./frameworks": "./src/frameworks.ts", "./measurements": "./src/measurements.ts", + "./comparison": "./src/comparison.ts", "./storage": "./src/storage.ts" }, "dependencies": { diff --git a/packages/results/src/comparison.ts b/packages/results/src/comparison.ts new file mode 100644 index 00000000..da7e9e1d --- /dev/null +++ b/packages/results/src/comparison.ts @@ -0,0 +1,125 @@ +import { isDeepStrictEqual } from 'node:util' +import { VARIANCE_THRESHOLD_PERCENT } from './measurements.ts' +import { + MetricBaseSchema, + type FrameworkResult, + type MetricBase, +} from './schemas.ts' + +function percentageChange(current: number, previous: number): number { + if (previous === 0) return 0 + const change = ((current - previous) / previous) * 100 + return Number.isFinite(change) ? change : 0 +} + +function record(value: unknown): Record | undefined { + return value !== null && typeof value === 'object' && !Array.isArray(value) + ? (value as Record) + : undefined +} + +export function metricSummaries( + value: unknown, + prefix = '', +): Map { + const summaries = new Map() + const object = record(value) + if (!object) return summaries + const metric = MetricBaseSchema.safeParse(object) + if (metric.success) { + summaries.set(prefix, value as MetricBase) + return summaries + } + for (const [key, child] of Object.entries(object)) { + if (key === 'variance' || key === 'iterations') continue + for (const [path, summary] of metricSummaries( + child, + prefix ? `${prefix}.${key}` : key, + )) + summaries.set(path, summary) + } + return summaries +} + +function benchmarkConfiguration(value: unknown) { + const object = record(value) + if (!object) return undefined + const keys = [ + 'name', + 'configuration', + 'source', + 'path', + 'durationMs', + 'warmupDurationMs', + 'minimumSamples', + 'warmupMinimumSamples', + 'bodySizeKb', + 'duplicationFactor', + ] + return Object.fromEntries( + keys.filter((key) => key in object).map((key) => [key, object[key]]), + ) +} + +export function withSummaryVariance( + current: FrameworkResult, + previous: FrameworkResult | null, + measuredTests = Object.keys(current), +): FrameworkResult { + const result = structuredClone(current) + const currentRecord = result as Record + const previousRecord = previous as Record | null + const measured = new Set(measuredTests) + const sameVersion = + Boolean(current.frameworkVersion) && + current.frameworkVersion !== 'unknown' && + current.frameworkVersion === previous?.frameworkVersion + + for (const [test, value] of Object.entries(currentRecord)) { + const summaries = metricSummaries(value) + const oldValue = previousRecord?.[test] + const oldSummaries = metricSummaries(oldValue) + for (const [path, summary] of summaries) { + delete summary.variance + if (!measured.has(test)) continue + const baseline = oldSummaries.get(path) + const browserTest = + test === 'clientSideRenderedTests' || test === 'serverSideRenderedTests' + const comparable = + sameVersion && + baseline && + current.runner === previous?.runner && + (!browserTest || current.browserVersion === previous?.browserVersion) && + isDeepStrictEqual( + benchmarkConfiguration(value), + benchmarkConfiguration(oldValue), + ) + summary.variance = { + comparedToMeasuredAt: comparable + ? (previous?.timingMeasuredAt ?? null) + : null, + meanPercent: comparable + ? percentageChange(summary.mean, baseline.mean) + : 0, + medianPercent: comparable + ? percentageChange(summary.median, baseline.median) + : 0, + minPercent: comparable + ? percentageChange(summary.min, baseline.min) + : 0, + maxPercent: comparable + ? percentageChange(summary.max, baseline.max) + : 0, + } + } + } + return result +} + +export function needsBenchmarkRerun(metric: MetricBase): boolean { + return ( + // Preserve the inclusive boundary despite floating-point arithmetic (0.9 vs 1). + Math.abs(metric.variance?.meanPercent ?? 0) >= + VARIANCE_THRESHOLD_PERCENT - Number.EPSILON * 100 + ) +} diff --git a/packages/results/src/measurements.ts b/packages/results/src/measurements.ts index 4304d1b9..83aec72d 100644 --- a/packages/results/src/measurements.ts +++ b/packages/results/src/measurements.ts @@ -1,3 +1,5 @@ +export const VARIANCE_THRESHOLD_PERCENT = 10 + /** Shared defaults for every framework. Scans run once; benchmarks repeat. */ export const measurements = { install: { runs: 5 }, diff --git a/packages/results/src/schemas.ts b/packages/results/src/schemas.ts index 3301fc40..740c2eac 100644 --- a/packages/results/src/schemas.ts +++ b/packages/results/src/schemas.ts @@ -1,11 +1,20 @@ import { selectRepresentativeIteration } from './selection.ts' import { z } from 'zod' +export const MetricVarianceSchema = z.object({ + comparedToMeasuredAt: z.string().nullable(), + meanPercent: z.number(), + medianPercent: z.number(), + minPercent: z.number(), + maxPercent: z.number(), +}) + export const MetricBaseSchema = z.object({ mean: z.number(), median: z.number(), min: z.number(), max: z.number(), + variance: MetricVarianceSchema.optional(), }) export type MetricBase = z.infer diff --git a/packages/stats-generator/package.json b/packages/stats-generator/package.json index 3bec65cd..3696dee5 100644 --- a/packages/stats-generator/package.json +++ b/packages/stats-generator/package.json @@ -22,6 +22,7 @@ "run:first-party-dependencies": "node src/run-first-party-dependency-scan.ts", "run:node-engines": "node src/run-node-engines-scan.ts", "validate:ci-stats": "node src/save-ci-stats.ts --validate-only --require-all-artifacts", + "check:benchmark-consistency": "node src/check-benchmark-consistency.ts", "save:ci-stats": "node src/save-ci-stats.ts", "generate:preview-comment": "node src/generate-preview-comment.ts", "validate": "node src/validate-stats.ts", diff --git a/packages/stats-generator/src/benchmark-consistency.ts b/packages/stats-generator/src/benchmark-consistency.ts new file mode 100644 index 00000000..e17344f4 --- /dev/null +++ b/packages/stats-generator/src/benchmark-consistency.ts @@ -0,0 +1,39 @@ +import type { CombinedResult } from '@framework-tracker/results/storage' +import { VARIANCE_THRESHOLD_PERCENT } from '@framework-tracker/results/measurements' +import { + metricSummaries, + needsBenchmarkRerun, +} from '@framework-tracker/results/comparison' + +export class BenchmarkConsistencyError extends Error { + constructor(changes: string[]) { + super( + [ + `Same-version benchmark means changed by ${VARIANCE_THRESHOLD_PERCENT}% or more:`, + ...changes, + 'Re-run the generate stats workflow. If the change persists, investigate before accepting a new baseline.', + ].join('\n'), + ) + this.name = 'BenchmarkConsistencyError' + } +} + +export function assertBenchmarkConsistency( + results: Iterable, +): void { + const changes: string[] = [] + for (const result of results) { + for (const [testName, test] of Object.entries(result.tests)) { + if (!test) continue + for (const [path, metric] of metricSummaries(test.result)) { + if (!needsBenchmarkRerun(metric)) continue + const percent = metric.variance!.meanPercent + const version = result.projects[test.project]?.frameworkVersion + changes.push( + `${result.framework}/${test.project}/${testName}${path ? `.${path}` : ''}: ${percent >= 0 ? '+' : ''}${percent.toFixed(2)}% (framework ${version} unchanged; threshold ±${VARIANCE_THRESHOLD_PERCENT}%).`, + ) + } + } + } + if (changes.length) throw new BenchmarkConsistencyError(changes) +} diff --git a/packages/stats-generator/src/check-benchmark-consistency.ts b/packages/stats-generator/src/check-benchmark-consistency.ts new file mode 100644 index 00000000..23fdd4e0 --- /dev/null +++ b/packages/stats-generator/src/check-benchmark-consistency.ts @@ -0,0 +1,88 @@ +import { globSync, readFileSync, statSync } from 'node:fs' +import { join } from 'node:path' +import { + CombinedResultSchema, + projectResult, + resultsRoot, + type CombinedResult, +} from '@framework-tracker/results/storage' +import { withSummaryVariance } from '@framework-tracker/results/comparison' +import { assertBenchmarkConsistency } from './benchmark-consistency.ts' + +export function checkStoredBenchmarkConsistency( + baselineRoot: string, + currentRoot = resultsRoot, +): void { + if (!statSync(baselineRoot).isDirectory()) + throw new Error('Expected a baseline results directory') + const files = [...globSync('*/latest.json', { cwd: currentRoot })] + if (!files.length) throw new Error('No current benchmark results found') + const comparisons: CombinedResult[] = [] + for (const file of files) { + const current = CombinedResultSchema.parse( + JSON.parse(readFileSync(join(currentRoot, file), 'utf8')), + ) + let baseline: CombinedResult | null = null + try { + baseline = CombinedResultSchema.parse( + JSON.parse(readFileSync(join(baselineRoot, file), 'utf8')), + ) + } catch (error) { + if (!( + error instanceof Error && + 'code' in error && + error.code === 'ENOENT' + )) + throw error + } + if (baseline && baseline.framework !== current.framework) + throw new Error(`Mismatched baseline framework: ${file}`) + const projects = Object.fromEntries( + (['starter', 'app'] as const) + .filter((project) => current.projects[project]) + .map((project) => [ + project, + withSummaryVariance( + projectResult(current, project), + baseline?.projects[project] + ? projectResult(baseline, project) + : null, + ), + ]), + ) + comparisons.push( + CombinedResultSchema.parse({ + ...current, + tests: Object.fromEntries( + Object.entries(current.tests).map(([name, test]) => [ + name, + test + ? { + ...test, + result: (projects[test.project] as Record)[ + name + ], + } + : undefined, + ]), + ), + }), + ) + } + assertBenchmarkConsistency(comparisons) +} + +if (import.meta.main) { + try { + const baselineRoot = process.argv[2] + if (!baselineRoot) + throw new Error( + 'Usage: check-benchmark-consistency ', + ) + checkStoredBenchmarkConsistency(baselineRoot) + console.info('Benchmark consistency check passed.') + } catch (error) { + console.error(error) + process.exitCode = 1 + } +} diff --git a/packages/stats-generator/src/save-ci-stats.ts b/packages/stats-generator/src/save-ci-stats.ts index b821ab58..d3cd42ab 100644 --- a/packages/stats-generator/src/save-ci-stats.ts +++ b/packages/stats-generator/src/save-ci-stats.ts @@ -18,7 +18,10 @@ import { DevServerStatsSchema, BaseResultSchema, InstallResultSchema, + FrameworkResultSchema, + type FrameworkResult, } from '@framework-tracker/results' +import { withSummaryVariance } from '@framework-tracker/results/comparison' import { validateSSRLoadForPersistence } from './ssrLoad/persistence.ts' import { mergeRequestThroughputArtifact, @@ -127,19 +130,31 @@ export async function processCIStats( } } - const projectStats = new Map() + const projectStats = new Map() const combinedStats = new Map() - function collect(packageName: string, stats: CIStats) { + function collect( + packageName: string, + stats: CIStats, + existing: CIStats | null, + ) { validateSSRLoadForPersistence(stats) validateRequestThroughputForPersistence(stats) const { framework } = resultLocation(packageName) + const measuredTests = Object.keys(stats).filter( + (key) => stats[key as keyof CIStats] !== existing?.[key as keyof CIStats], + ) + const comparedStats = withSummaryVariance( + FrameworkResultSchema.parse(stats), + FrameworkResultSchema.nullable().parse(existing), + measuredTests, + ) combinedStats.set( framework, mergeProjectResult(combinedStats.get(framework) ?? null, packageName, { - ...stats, + ...comparedStats, }), ) - projectStats.set(packageName, stats) + projectStats.set(packageName, comparedStats) } for (const framework of frameworks) { @@ -418,7 +433,7 @@ export async function processCIStats( releasedAt, ) - collect(packageName, stats) + collect(packageName, stats, existingStats) console.info('') } @@ -607,7 +622,7 @@ export async function processCIStats( releasedAt, ) - collect(packageName, stats) + collect(packageName, stats, existingStats) console.info('') }