From 2c62ca34fc81e427462a80d8804ad532dea14d85 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 2 Nov 2026 09:05:30 +0530 Subject: [PATCH 1/4] ci(cli): judge block-scoped benchmark timings by per-block agreement Block-scoped workloads now run in blocks of two launches, so each build gets five stack sessions at ten runs. Launches record their block index, and the timing verdict needs a strict majority of paired blocks to move the same way past the noise floor before it reads slower or faster. Co-Authored-By: Claude Opus 5.5 --- apps/cli/docs/tracing-monitoring.md | 5 +- .../cli/scripts/bench-cli.integration.test.ts | 16 +++-- apps/cli/scripts/bench-cli.ts | 17 +++-- apps/cli/scripts/bench-cli.unit.test.ts | 15 +++++ apps/cli/scripts/bench-report.ts | 59 +++++++++++++++-- apps/cli/scripts/bench-report.unit.test.ts | 66 +++++++++++++++++++ 6 files changed, 163 insertions(+), 15 deletions(-) diff --git a/apps/cli/docs/tracing-monitoring.md b/apps/cli/docs/tracing-monitoring.md index cdc4b92766..6247f162e9 100644 --- a/apps/cli/docs/tracing-monitoring.md +++ b/apps/cli/docs/tracing-monitoring.md @@ -181,6 +181,9 @@ the timing ranges, API calls per command, and the slowest phases. It reports: - Wall time: base ("before") and branch ("after") medians and ranges. A verdict of slower or faster needs the branch median outside the base min-max range, a change of at least 5 percent and 5 ms, and at least 5 measured runs; otherwise it reads "within noise", or "not comparable" when a build failed. + Block-scoped workloads record a block index per launch and, from three paired blocks, replace the + range check with a majority rule: more than half of the blocks must move the same way past the + same floor, so one outlier stack session cannot flag a workload. Thresholds are named constants in `bench-report.ts`. - API calls per workload, base and branch. - Phase deltas: the three largest span deltas for each workload with a slower or faster verdict. @@ -297,7 +300,7 @@ described above: | Field | Meaning | | ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------- | | `setup`, `teardown` | Lists of CLI arguments run untimed with the build under test. `teardown` always runs, even after a failure or timeout. | -| `setupScope` | `run` (default) repeats setup and teardown for every launch; `block` starts one stack per block of five launches. | +| `setupScope` | `run` (default) repeats setup and teardown for every launch; `block` starts one stack per block of two launches. | | `stack` | Sets `SUPABASE_EXPERIMENTAL_STACK=1` on every launch of the workload, the only variable a workload can set. | | `runtime` | `docker` or `native`. A stack workload must pass the same `--runtime` to every `stack start`. | | `fixture` | A directory under `apps/cli/bench/fixtures/` copied into the project's `supabase/` directory after `init`. | diff --git a/apps/cli/scripts/bench-cli.integration.test.ts b/apps/cli/scripts/bench-cli.integration.test.ts index 6744dbe420..0254e056bd 100644 --- a/apps/cli/scripts/bench-cli.integration.test.ts +++ b/apps/cli/scripts/bench-cli.integration.test.ts @@ -204,13 +204,21 @@ describe("runWorkloadBench", () => { expect(report.base).toHaveLength(6); expect(report.branch).toHaveLength(6); const boots = calls.filter((call) => call.command === "boot"); - expect(boots.map((call) => call.build)).toEqual(["base", "branch", "branch", "base"]); - expect(calls.filter((call) => call.command === "halt")).toHaveLength(4); - // One warmup plus five measured launches share the first block's stack. + expect(boots.map((call) => call.build)).toEqual([ + "base", + "branch", + "branch", + "base", + "base", + "branch", + ]); + expect(calls.filter((call) => call.command === "halt")).toHaveLength(6); + expect(report.base.map((sample) => sample.block)).toEqual([0, 0, 1, 1, 2, 2]); + // One warmup plus two measured launches share the first block's stack. const firstBlock = calls.filter((call) => call.home === boots[0]?.home); expect(firstBlock.map((call) => call.command)).toEqual([ "boot", - ...Array.from({ length: 6 }, () => "work"), + ...Array.from({ length: 3 }, () => "work"), "halt", ]); }), diff --git a/apps/cli/scripts/bench-cli.ts b/apps/cli/scripts/bench-cli.ts index a048619fa6..5f8cc7c8ca 100644 --- a/apps/cli/scripts/bench-cli.ts +++ b/apps/cli/scripts/bench-cli.ts @@ -463,11 +463,13 @@ interface LaunchOutcome { /** Planned units of work for one build: a run-scoped unit holds one launch, a block-scoped one many. */ export interface PlannedUnit { readonly build: Build; + /** Block index for block-scoped units. */ + readonly block?: number; readonly launches: ReadonlyArray<"warmup" | "timed">; } /** Measured launches per block for `block`-scoped workloads. */ -export const BLOCK_SIZE = 5; +export const BLOCK_SIZE = 2; /** * Units in execution order. `run` scope reuses {@link buildRunPlan}, one launch per unit. @@ -495,7 +497,7 @@ export function buildUnitPlan( ...Array.from({ length: measured }, () => "timed" as const), ]; const order: ReadonlyArray = block % 2 === 0 ? ["base", "branch"] : ["branch", "base"]; - for (const build of order) units.push({ build, launches }); + for (const build of order) units.push({ build, block, launches }); } return units; } @@ -644,7 +646,7 @@ export const runWorkloadBench = ( const plan = buildUnitPlan(runs, warmup, scope); const started = yield* Clock.currentTimeNanos; - const timed: Record> = { + const timed: Record> = { base: [], branch: [], }; @@ -668,10 +670,14 @@ export const runWorkloadBench = ( (options.cpuProf && build === "branch" && !alreadyProfiled), ); - const record = (build: Build, outcomes: ReadonlyArray) => { + const record = (build: Build, outcomes: ReadonlyArray, block?: number) => { for (const outcome of outcomes) { if (outcome.kind === "timed") { - timed[build].push({ wallMs: outcome.wallMs, exitCode: outcome.exitCode }); + timed[build].push({ + wallMs: outcome.wallMs, + exitCode: outcome.exitCode, + ...(block === undefined ? {} : { block }), + }); } else if (outcome.kind === "traced") { traced[build].push({ exitCode: outcome.exitCode, trace: outcome.trace }); } @@ -728,6 +734,7 @@ export const runWorkloadBench = ( teardownFailures, onProfile, }), + planned.block, ); } diff --git a/apps/cli/scripts/bench-cli.unit.test.ts b/apps/cli/scripts/bench-cli.unit.test.ts index 15f8b80855..f577d8aa74 100644 --- a/apps/cli/scripts/bench-cli.unit.test.ts +++ b/apps/cli/scripts/bench-cli.unit.test.ts @@ -137,6 +137,21 @@ describe("buildUnitPlan", () => { }); }); +describe("buildUnitPlan block sizing", () => { + it("gives ten runs five blocks per build by default, each tagged with its block index", () => { + const plan = buildUnitPlan(10, 1, "block"); + + for (const build of ["base", "branch"] as const) { + expect(plan.filter((unit) => unit.build === build).map((unit) => unit.block)).toEqual([ + 0, 1, 2, 3, 4, + ]); + } + expect( + plan.every((unit) => unit.launches.filter((kind) => kind === "timed").length === 2), + ).toBe(true); + }); +}); + describe("parseBenchArgs for stack runs", () => { it("lets explicit --runs and --warmup override the manifest, and defaults otherwise", () => { expect(parseBenchArgs(required)).toMatchObject({ diff --git a/apps/cli/scripts/bench-report.ts b/apps/cli/scripts/bench-report.ts index a866d304d2..07611cf94b 100644 --- a/apps/cli/scripts/bench-report.ts +++ b/apps/cli/scripts/bench-report.ts @@ -289,6 +289,8 @@ const TraceSample = Schema.Struct({ const RunSample = Schema.Struct({ wallMs: Schema.Finite, + /** Block index of a launch from a block-scoped workload; absent for run-scoped launches. */ + block: Schema.optionalKey(Schema.Finite), exitCode: Schema.Finite, /** Exit code of the traced launch, recorded even when no spans could be decoded. */ tracedExitCode: Schema.optionalKey(Schema.Finite), @@ -345,29 +347,75 @@ export const encodeBenchReport = Schema.encodeEffect(BenchReportJson); export const VERDICT_MIN_PCT = 5; export const VERDICT_MIN_MS = 5; export const VERDICT_MIN_SAMPLES = 5; +/** Paired blocks needed before the per-block agreement rule replaces the min-max range check. */ +export const VERDICT_MIN_BLOCKS = 3; export const PHASE_ROWS = 3; export type TimingVerdict = "slower" | "faster" | "within noise" | "not comparable"; +/** Median wall time of one block for each build, paired by block index. */ +export interface BlockPair { + readonly base: number; + readonly branch: number; +} + +const exceedsFloor = (delta: Delta): boolean => + Math.abs(delta.pct) >= VERDICT_MIN_PCT && Math.abs(delta.ms) >= VERDICT_MIN_MS; + /** - * "slower" or "faster" only when the branch median is outside the base min-max range and the - * change is at least {@link VERDICT_MIN_PCT} percent and {@link VERDICT_MIN_MS} ms. + * "slower" or "faster" only when the median change is at least {@link VERDICT_MIN_PCT} percent + * and {@link VERDICT_MIN_MS} ms. Run-scoped samples also need the branch median outside the base + * min-max range. With at least {@link VERDICT_MIN_BLOCKS} paired blocks, a strict majority of + * blocks must instead move the same way, each past the same floor. */ export function timingVerdict( base: TimingSummary, branch: TimingSummary, - context: { readonly comparable: boolean; readonly samples: number }, + context: { + readonly comparable: boolean; + readonly samples: number; + readonly blocks?: ReadonlyArray; + }, ): TimingVerdict { if (!context.comparable || context.samples < VERDICT_MIN_SAMPLES) return "not comparable"; const delta = deltaOf(base.median, branch.median); - if (Math.abs(delta.pct) < VERDICT_MIN_PCT || Math.abs(delta.ms) < VERDICT_MIN_MS) { - return "within noise"; + if (!exceedsFloor(delta)) return "within noise"; + const direction = delta.ms > 0 ? "slower" : "faster"; + const blocks = context.blocks; + if (blocks !== undefined && blocks.length >= VERDICT_MIN_BLOCKS) { + const agreeing = blocks.filter((block) => { + const blockDelta = deltaOf(block.base, block.branch); + return exceedsFloor(blockDelta) && blockDelta.ms > 0 === delta.ms > 0; + }).length; + return agreeing * 2 > blocks.length ? direction : "within noise"; } if (branch.median > base.max) return "slower"; if (branch.median < base.min) return "faster"; return "within noise"; } +/** Pairs the per-block medians of both builds; empty unless every sample carries a block index. */ +export function pairBlocks( + base: ReadonlyArray, + branch: ReadonlyArray, +): ReadonlyArray { + const group = (samples: ReadonlyArray): Map> | undefined => { + const blocks = new Map>(); + for (const sample of samples) { + if (sample.block === undefined) return undefined; + blocks.set(sample.block, [...(blocks.get(sample.block) ?? []), sample.wallMs]); + } + return blocks; + }; + const baseBlocks = group(base); + const branchBlocks = group(branch); + if (baseBlocks === undefined || branchBlocks === undefined) return []; + return [...baseBlocks.entries()].flatMap(([block, values]) => { + const other = branchBlocks.get(block); + return other === undefined ? [] : [{ base: median(values), branch: median(other) }]; + }); +} + export interface ApiSummary { readonly medianCalls: number; readonly runCalls: ReadonlyArray; @@ -559,6 +607,7 @@ export function analyzeWorkload(workload: WorkloadReport, traced: boolean): Work const verdict = timingVerdict(base.timing, branch.timing, { comparable: !differ && runsCleanly(base) && runsCleanly(branch), samples: Math.min(base.samples, branch.samples), + blocks: pairBlocks(workload.base, workload.branch), }); const spans = base.spans === undefined || branch.spans === undefined diff --git a/apps/cli/scripts/bench-report.unit.test.ts b/apps/cli/scripts/bench-report.unit.test.ts index 9f41a41d98..e6ee731ef4 100644 --- a/apps/cli/scripts/bench-report.unit.test.ts +++ b/apps/cli/scripts/bench-report.unit.test.ts @@ -13,6 +13,7 @@ import { parseWorkloadManifest, summarizeSpanRun, summarizeTimings, + pairBlocks, timingVerdict, validateWorkloads, type BenchReport, @@ -210,6 +211,71 @@ describe("timingVerdict", () => { }); }); +describe("timingVerdict with blocks", () => { + const base = { median: 100, min: 60, max: 150, p90: 140 }; + const context = { comparable: true, samples: 10 }; + const pairs = (...branchMedians: ReadonlyArray) => + branchMedians.map((branch) => ({ base: 100, branch })); + + it("reads mixed-sign blocks as within noise even when the pooled median moved", () => { + const branch = { median: 130, min: 60, max: 200, p90: 190 }; + expect( + timingVerdict(base, branch, { + ...context, + blocks: pairs(180, 70, 130, 75, 85), + }), + ).toBe("within noise"); + }); + + it("flags slower when a majority of blocks agree past the floor", () => { + const branch = { median: 130, min: 100, max: 150, p90: 150 }; + expect( + timingVerdict(base, branch, { + ...context, + blocks: pairs(130, 125, 140, 99, 135), + }), + ).toBe("slower"); + }); + + it("flags faster when a majority of blocks agree past the floor", () => { + const branch = { median: 70, min: 60, max: 100, p90: 95 }; + expect( + timingVerdict(base, branch, { + ...context, + blocks: pairs(70, 65, 75, 110, 72), + }), + ).toBe("faster"); + }); + + it("falls back to the range check below three blocks", () => { + const branch = { median: 120, min: 100, max: 130, p90: 130 }; + expect(timingVerdict(base, branch, { ...context, blocks: pairs(120, 120) })).toBe( + "within noise", + ); + }); +}); + +describe("pairBlocks", () => { + const at = (block: number, wallMs: number) => ({ + wallMs, + exitCode: 0, + block, + }); + + it("pairs per-block medians of both builds by block index", () => { + expect( + pairBlocks([at(0, 10), at(0, 20), at(1, 30)], [at(0, 40), at(0, 60), at(1, 50)]), + ).toEqual([ + { base: 15, branch: 50 }, + { base: 30, branch: 50 }, + ]); + }); + + it("returns no pairs when a sample has no block index", () => { + expect(pairBlocks([{ wallMs: 10, exitCode: 0 }], [at(0, 10)])).toEqual([]); + }); +}); + const sample = ( wallMs: number, api?: { From 4573f6793615caee35653b912ee00eb2c3063c5e Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 2 Nov 2026 11:42:07 +0530 Subject: [PATCH 2/4] ci(cli): rescale block-scoped benchmark estimates for blocks of two Co-Authored-By: Claude Opus 5.5 --- apps/cli/bench/workloads.full.json | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/cli/bench/workloads.full.json b/apps/cli/bench/workloads.full.json index 90fef53a28..0b17661da1 100644 --- a/apps/cli/bench/workloads.full.json +++ b/apps/cli/bench/workloads.full.json @@ -48,7 +48,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 90 + "estimatedSeconds": 220 }, { "id": "db-query", @@ -61,7 +61,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 90 + "estimatedSeconds": 220 }, { "id": "db-reset", @@ -90,7 +90,7 @@ "warmup": 1, "traceRuns": 1, "timeoutSeconds": 120, - "estimatedSeconds": 60 + "estimatedSeconds": 180 }, { "id": "gen-types-small", @@ -104,7 +104,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 90 + "estimatedSeconds": 220 }, { "id": "gen-types-medium", @@ -118,7 +118,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 90 + "estimatedSeconds": 220 }, { "id": "gen-types-large", @@ -132,7 +132,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 100 + "estimatedSeconds": 250 }, { "id": "db-diff-cold", @@ -183,7 +183,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 100 + "estimatedSeconds": 250 }, { "id": "db-lint", @@ -197,7 +197,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 90 + "estimatedSeconds": 220 }, { "id": "migration-up", @@ -239,7 +239,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 90 + "estimatedSeconds": 220 }, { "id": "stack-start-docker", @@ -266,7 +266,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 60 + "estimatedSeconds": 150 }, { "id": "stack-stop-docker", @@ -294,7 +294,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 60 + "estimatedSeconds": 150 }, { "id": "stack-db-reset-docker", @@ -336,7 +336,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 60 + "estimatedSeconds": 150 }, { "id": "stack-stop-native", @@ -364,7 +364,7 @@ "runs": 10, "warmup": 1, "traceRuns": 2, - "estimatedSeconds": 60 + "estimatedSeconds": 150 }, { "id": "stack-db-reset-native", From d848714d7d069b99e1f8cf636595e2e594f69757 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 2 Nov 2026 12:31:14 +0530 Subject: [PATCH 3/4] ci(cli): require each build's median outside the other's range for a timing verdict Co-Authored-By: Claude Opus 5.5 --- apps/cli/docs/tracing-monitoring.md | 10 ++-- apps/cli/scripts/bench-report.ts | 20 ++++---- apps/cli/scripts/bench-report.unit.test.ts | 60 ++++++++++++---------- 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/apps/cli/docs/tracing-monitoring.md b/apps/cli/docs/tracing-monitoring.md index 6247f162e9..08c3e138d1 100644 --- a/apps/cli/docs/tracing-monitoring.md +++ b/apps/cli/docs/tracing-monitoring.md @@ -178,12 +178,12 @@ the timing ranges, API calls per command, and the slowest phases. It reports: responses (the build did not follow the recorded scenario), non-zero or differing exit codes, missing traces, and too few runs for a timing verdict. Other count changes, such as 1 to 2 calls, appear in the API calls table with the changed endpoints but are not flagged. -- Wall time: base ("before") and branch ("after") medians and ranges. A verdict of slower or faster needs the branch - median outside the base min-max range, a change of at least 5 percent and 5 ms, and at least 5 +- Wall time: base ("before") and branch ("after") medians and ranges. A verdict of slower or faster needs each build's + median outside the other's min-max range, a change of at least 5 percent and 5 ms, and at least 5 measured runs; otherwise it reads "within noise", or "not comparable" when a build failed. - Block-scoped workloads record a block index per launch and, from three paired blocks, replace the - range check with a majority rule: more than half of the blocks must move the same way past the - same floor, so one outlier stack session cannot flag a workload. + Block-scoped workloads record a block index per launch and, from three paired blocks, also need a + majority rule: more than half of the blocks must move the same way past the same floor, so one + outlier stack session cannot flag a workload. Thresholds are named constants in `bench-report.ts`. - API calls per workload, base and branch. - Phase deltas: the three largest span deltas for each workload with a slower or faster verdict. diff --git a/apps/cli/scripts/bench-report.ts b/apps/cli/scripts/bench-report.ts index 07611cf94b..5508628f12 100644 --- a/apps/cli/scripts/bench-report.ts +++ b/apps/cli/scripts/bench-report.ts @@ -364,9 +364,9 @@ const exceedsFloor = (delta: Delta): boolean => /** * "slower" or "faster" only when the median change is at least {@link VERDICT_MIN_PCT} percent - * and {@link VERDICT_MIN_MS} ms. Run-scoped samples also need the branch median outside the base - * min-max range. With at least {@link VERDICT_MIN_BLOCKS} paired blocks, a strict majority of - * blocks must instead move the same way, each past the same floor. + * and {@link VERDICT_MIN_MS} ms and each build's median lies outside the other's min-max range, so + * a wide spread in either build reads as noise. With at least {@link VERDICT_MIN_BLOCKS} paired + * blocks, a strict majority of blocks must also move the same way, each past the same floor. */ export function timingVerdict( base: TimingSummary, @@ -380,18 +380,20 @@ export function timingVerdict( if (!context.comparable || context.samples < VERDICT_MIN_SAMPLES) return "not comparable"; const delta = deltaOf(base.median, branch.median); if (!exceedsFloor(delta)) return "within noise"; - const direction = delta.ms > 0 ? "slower" : "faster"; + const slower = delta.ms > 0; + const separated = slower + ? branch.median > base.max && base.median < branch.min + : branch.median < base.min && base.median > branch.max; + if (!separated) return "within noise"; const blocks = context.blocks; if (blocks !== undefined && blocks.length >= VERDICT_MIN_BLOCKS) { const agreeing = blocks.filter((block) => { const blockDelta = deltaOf(block.base, block.branch); - return exceedsFloor(blockDelta) && blockDelta.ms > 0 === delta.ms > 0; + return exceedsFloor(blockDelta) && blockDelta.ms > 0 === slower; }).length; - return agreeing * 2 > blocks.length ? direction : "within noise"; + if (agreeing * 2 <= blocks.length) return "within noise"; } - if (branch.median > base.max) return "slower"; - if (branch.median < base.min) return "faster"; - return "within noise"; + return slower ? "slower" : "faster"; } /** Pairs the per-block medians of both builds; empty unless every sample carries a block index. */ diff --git a/apps/cli/scripts/bench-report.unit.test.ts b/apps/cli/scripts/bench-report.unit.test.ts index e6ee731ef4..d0654f41a8 100644 --- a/apps/cli/scripts/bench-report.unit.test.ts +++ b/apps/cli/scripts/bench-report.unit.test.ts @@ -212,46 +212,52 @@ describe("timingVerdict", () => { }); describe("timingVerdict with blocks", () => { - const base = { median: 100, min: 60, max: 150, p90: 140 }; + const base = { median: 100, min: 90, max: 110, p90: 108 }; const context = { comparable: true, samples: 10 }; const pairs = (...branchMedians: ReadonlyArray) => branchMedians.map((branch) => ({ base: 100, branch })); - it("reads mixed-sign blocks as within noise even when the pooled median moved", () => { - const branch = { median: 130, min: 60, max: 200, p90: 190 }; - expect( - timingVerdict(base, branch, { - ...context, - blocks: pairs(180, 70, 130, 75, 85), - }), - ).toBe("within noise"); + it("reads mixed-sign blocks as within noise although the ranges are separated", () => { + const branch = { median: 130, min: 115, max: 200, p90: 190 }; + expect(timingVerdict(base, branch, { ...context, blocks: pairs(180, 70, 130, 75, 85) })).toBe( + "within noise", + ); + expect(timingVerdict(base, branch, context)).toBe("slower"); }); it("flags slower when a majority of blocks agree past the floor", () => { - const branch = { median: 130, min: 100, max: 150, p90: 150 }; - expect( - timingVerdict(base, branch, { - ...context, - blocks: pairs(130, 125, 140, 99, 135), - }), - ).toBe("slower"); + const branch = { median: 130, min: 115, max: 150, p90: 150 }; + expect(timingVerdict(base, branch, { ...context, blocks: pairs(130, 125, 140, 99, 135) })).toBe( + "slower", + ); }); it("flags faster when a majority of blocks agree past the floor", () => { - const branch = { median: 70, min: 60, max: 100, p90: 95 }; - expect( - timingVerdict(base, branch, { - ...context, - blocks: pairs(70, 65, 75, 110, 72), - }), - ).toBe("faster"); + const branch = { median: 70, min: 60, max: 85, p90: 85 }; + expect(timingVerdict(base, branch, { ...context, blocks: pairs(70, 65, 75, 110, 72) })).toBe( + "faster", + ); }); it("falls back to the range check below three blocks", () => { - const branch = { median: 120, min: 100, max: 130, p90: 130 }; - expect(timingVerdict(base, branch, { ...context, blocks: pairs(120, 120) })).toBe( - "within noise", - ); + const branch = { median: 130, min: 115, max: 150, p90: 150 }; + expect(timingVerdict(base, branch, { ...context, blocks: pairs(60, 60) })).toBe("slower"); + }); +}); + +describe("timingVerdict with a wider build", () => { + const context = { comparable: true, samples: 10 }; + + it("reads a slower median as noise when the base median sits inside the branch range", () => { + const base = { median: 225, min: 218, max: 240, p90: 238 }; + const branch = { median: 274, min: 218, max: 305, p90: 300 }; + expect(timingVerdict(base, branch, context)).toBe("within noise"); + }); + + it("reads a faster median as noise when the base median sits inside the branch range", () => { + const base = { median: 11211, min: 11172, max: 11224, p90: 11224 }; + const branch = { median: 10557, min: 10381, max: 11236, p90: 11236 }; + expect(timingVerdict(base, branch, context)).toBe("within noise"); }); }); From 628df346f3c5b25f7854f5637f3f339a199205b8 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 2 Nov 2026 17:05:28 +0530 Subject: [PATCH 4/4] ci(cli): judge benchmark timings against interquartile spread Block-scoped workloads judge the median of per-block deltas against the block medians' spread, so one odd stack session can neither flag nor hide a change. Run-scoped workloads keep the base range check and also need the change to clear the launches' spread. Co-Authored-By: Claude Opus 5.5 --- apps/cli/docs/tracing-monitoring.md | 14 +-- apps/cli/scripts/bench-report.ts | 47 ++++++--- apps/cli/scripts/bench-report.unit.test.ts | 106 ++++++++++++--------- 3 files changed, 100 insertions(+), 67 deletions(-) diff --git a/apps/cli/docs/tracing-monitoring.md b/apps/cli/docs/tracing-monitoring.md index 08c3e138d1..c42cee4b1e 100644 --- a/apps/cli/docs/tracing-monitoring.md +++ b/apps/cli/docs/tracing-monitoring.md @@ -178,12 +178,14 @@ the timing ranges, API calls per command, and the slowest phases. It reports: responses (the build did not follow the recorded scenario), non-zero or differing exit codes, missing traces, and too few runs for a timing verdict. Other count changes, such as 1 to 2 calls, appear in the API calls table with the changed endpoints but are not flagged. -- Wall time: base ("before") and branch ("after") medians and ranges. A verdict of slower or faster needs each build's - median outside the other's min-max range, a change of at least 5 percent and 5 ms, and at least 5 - measured runs; otherwise it reads "within noise", or "not comparable" when a build failed. - Block-scoped workloads record a block index per launch and, from three paired blocks, also need a - majority rule: more than half of the blocks must move the same way past the same floor, so one - outlier stack session cannot flag a workload. +- Wall time: base ("before") and branch ("after") medians and ranges. A verdict of slower or faster needs a + change of at least 5 percent and 5 ms that is also larger than 1.5 times the wider build's + interquartile range, a branch median beyond the base min-max range for run-scoped workloads, and + at least 5 measured runs; otherwise it reads "within noise", or "not + comparable" when a build failed. Block-scoped workloads record a block index per launch and, from + three paired blocks, judge block medians instead: the median per-block change must pass the same + tests and more than half of the blocks must move the same way past the floor, so one outlier + stack session can neither flag nor hide a change. Thresholds are named constants in `bench-report.ts`. - API calls per workload, base and branch. - Phase deltas: the three largest span deltas for each workload with a slower or faster verdict. diff --git a/apps/cli/scripts/bench-report.ts b/apps/cli/scripts/bench-report.ts index 5508628f12..dede4dc6a0 100644 --- a/apps/cli/scripts/bench-report.ts +++ b/apps/cli/scripts/bench-report.ts @@ -29,8 +29,13 @@ export interface TimingSummary { readonly min: number; readonly max: number; readonly p90: number; + readonly iqr: number; } +/** Interquartile range by nearest rank, so one extreme value at either end cannot move it. */ +const iqr = (values: ReadonlyArray): number => + percentile(values, 75) - percentile(values, 25); + export function summarizeTimings(values: ReadonlyArray): TimingSummary { const sorted = [...values].sort((a, b) => a - b); return { @@ -38,6 +43,7 @@ export function summarizeTimings(values: ReadonlyArray): TimingSummary { min: round(sorted[0] ?? 0), max: round(sorted[sorted.length - 1] ?? 0), p90: round(percentile(sorted, 90)), + iqr: round(iqr(sorted)), }; } @@ -347,8 +353,9 @@ export const encodeBenchReport = Schema.encodeEffect(BenchReportJson); export const VERDICT_MIN_PCT = 5; export const VERDICT_MIN_MS = 5; export const VERDICT_MIN_SAMPLES = 5; -/** Paired blocks needed before the per-block agreement rule replaces the min-max range check. */ +/** Paired blocks needed before the block, not the launch, is the unit of evidence for a verdict. */ export const VERDICT_MIN_BLOCKS = 3; +export const VERDICT_IQR_FACTOR = 1.5; export const PHASE_ROWS = 3; export type TimingVerdict = "slower" | "faster" | "within noise" | "not comparable"; @@ -363,10 +370,12 @@ const exceedsFloor = (delta: Delta): boolean => Math.abs(delta.pct) >= VERDICT_MIN_PCT && Math.abs(delta.ms) >= VERDICT_MIN_MS; /** - * "slower" or "faster" only when the median change is at least {@link VERDICT_MIN_PCT} percent - * and {@link VERDICT_MIN_MS} ms and each build's median lies outside the other's min-max range, so - * a wide spread in either build reads as noise. With at least {@link VERDICT_MIN_BLOCKS} paired - * blocks, a strict majority of blocks must also move the same way, each past the same floor. + * "slower" or "faster" only when the median change passes the {@link VERDICT_MIN_PCT} percent and + * {@link VERDICT_MIN_MS} ms floor and is larger than {@link VERDICT_IQR_FACTOR} times the wider + * build's interquartile range, so one extreme sample cannot fake a change. Run-scoped timings + * also need the branch median beyond the base range; with at least {@link VERDICT_MIN_BLOCKS} + * paired blocks the spread test runs on block medians instead, and a strict majority of blocks + * must pass the floor in the same direction. */ export function timingVerdict( base: TimingSummary, @@ -378,22 +387,32 @@ export function timingVerdict( }, ): TimingVerdict { if (!context.comparable || context.samples < VERDICT_MIN_SAMPLES) return "not comparable"; - const delta = deltaOf(base.median, branch.median); - if (!exceedsFloor(delta)) return "within noise"; - const slower = delta.ms > 0; - const separated = slower - ? branch.median > base.max && base.median < branch.min - : branch.median < base.min && base.median > branch.max; - if (!separated) return "within noise"; const blocks = context.blocks; if (blocks !== undefined && blocks.length >= VERDICT_MIN_BLOCKS) { + const baseMedians = blocks.map((block) => block.base); + const branchMedians = blocks.map((block) => block.branch); + const reference = median(baseMedians); + const ms = median(blocks.map((block) => block.branch - block.base)); + const pct = reference === 0 ? 0 : (ms / reference) * 100; + if (!exceedsFloor({ ms: round(ms), pct: round(pct) })) return "within noise"; const agreeing = blocks.filter((block) => { const blockDelta = deltaOf(block.base, block.branch); - return exceedsFloor(blockDelta) && blockDelta.ms > 0 === slower; + return exceedsFloor(blockDelta) && blockDelta.ms > 0 === ms > 0; }).length; if (agreeing * 2 <= blocks.length) return "within noise"; + if (Math.abs(ms) <= VERDICT_IQR_FACTOR * Math.max(iqr(baseMedians), iqr(branchMedians))) { + return "within noise"; + } + return ms > 0 ? "slower" : "faster"; + } + const delta = deltaOf(base.median, branch.median); + if (!exceedsFloor(delta)) return "within noise"; + const outsideRange = delta.ms > 0 ? branch.median > base.max : branch.median < base.min; + if (!outsideRange) return "within noise"; + if (Math.abs(delta.ms) <= VERDICT_IQR_FACTOR * Math.max(base.iqr, branch.iqr)) { + return "within noise"; } - return slower ? "slower" : "faster"; + return delta.ms > 0 ? "slower" : "faster"; } /** Pairs the per-block medians of both builds; empty unless every sample carries a block index. */ diff --git a/apps/cli/scripts/bench-report.unit.test.ts b/apps/cli/scripts/bench-report.unit.test.ts index d0654f41a8..00743cebae 100644 --- a/apps/cli/scripts/bench-report.unit.test.ts +++ b/apps/cli/scripts/bench-report.unit.test.ts @@ -52,6 +52,7 @@ describe("summarizeTimings", () => { min: 10, max: 50, p90: 50, + iqr: 20, }); }); }); @@ -181,83 +182,94 @@ describe("classifyHttpSpans", () => { }); }); +const timings = (values: ReadonlyArray) => summarizeTimings(values); +const repeat = (count: number, value: number) => Array.from({ length: count }, () => value); + describe("timingVerdict", () => { - const base = { median: 100, min: 95, max: 108, p90: 107 }; const context = { comparable: true, samples: 10 }; it.each([ - { branch: { median: 120, min: 115, max: 125, p90: 124 }, expected: "slower" }, - { branch: { median: 80, min: 78, max: 90, p90: 88 }, expected: "faster" }, - { branch: { median: 103, min: 99, max: 110, p90: 109 }, expected: "within noise" }, - { branch: { median: 107, min: 99, max: 118, p90: 117 }, expected: "within noise" }, - ])("calls a branch median of $branch.median $expected", ({ branch, expected }) => { - expect(timingVerdict(base, branch, context)).toBe(expected); + { branch: 120, expected: "slower" }, + { branch: 80, expected: "faster" }, + { branch: 103, expected: "within noise" }, + ])("calls a steady branch of $branch ms $expected", ({ branch, expected }) => { + expect(timingVerdict(timings(repeat(10, 100)), timings(repeat(10, branch)), context)).toBe( + expected, + ); }); it("ignores a large percentage change that is under the absolute floor", () => { - expect( - timingVerdict( - { median: 10, min: 9, max: 11, p90: 11 }, - { median: 14, min: 13, max: 15, p90: 15 }, - context, - ), - ).toBe("within noise"); + expect(timingVerdict(timings(repeat(10, 10)), timings(repeat(10, 14)), context)).toBe( + "within noise", + ); }); it("declines to compare when runs are not comparable or too few", () => { - const slow = { median: 200, min: 190, max: 210, p90: 209 }; + const base = timings(repeat(10, 100)); + const slow = timings(repeat(10, 200)); expect(timingVerdict(base, slow, { comparable: false, samples: 10 })).toBe("not comparable"); expect(timingVerdict(base, slow, { comparable: true, samples: 3 })).toBe("not comparable"); }); + + it("flags slower although one branch launch was fast", () => { + const branch = timings([...repeat(9, 125), 90]); + expect(timingVerdict(timings(repeat(10, 100)), branch, context)).toBe("slower"); + }); + + it("reads a branch median inside a noisy base range as noise on real five-run data", () => { + const base = timings([328, 234, 228, 229, 237]); + const branch = timings([303, 317, 273, 268, 313]); + expect(timingVerdict(base, branch, { comparable: true, samples: 5 })).toBe("within noise"); + }); + + it("reads a change past the floor but within 1.5 times the wider spread as noise", () => { + const base = timings([88, 92, 95, 100, 100, 100, 103, 107, 110, 112]); + const branch = timings([105, 110, 112, 115, 115, 115, 118, 120, 122, 125]); + expect(base.iqr).toBe(12); + expect(branch.iqr).toBe(8); + expect(branch.median).toBeGreaterThan(base.max); + expect(timingVerdict(base, branch, context)).toBe("within noise"); + }); }); describe("timingVerdict with blocks", () => { - const base = { median: 100, min: 90, max: 110, p90: 108 }; const context = { comparable: true, samples: 10 }; + const unused = timings(repeat(10, 100)); const pairs = (...branchMedians: ReadonlyArray) => branchMedians.map((branch) => ({ base: 100, branch })); - it("reads mixed-sign blocks as within noise although the ranges are separated", () => { - const branch = { median: 130, min: 115, max: 200, p90: 190 }; - expect(timingVerdict(base, branch, { ...context, blocks: pairs(180, 70, 130, 75, 85) })).toBe( - "within noise", - ); - expect(timingVerdict(base, branch, context)).toBe("slower"); - }); - - it("flags slower when a majority of blocks agree past the floor", () => { - const branch = { median: 130, min: 115, max: 150, p90: 150 }; - expect(timingVerdict(base, branch, { ...context, blocks: pairs(130, 125, 140, 99, 135) })).toBe( - "slower", - ); + it("flags slower when four of five blocks are clearly slower and one is fast", () => { + expect( + timingVerdict(unused, unused, { ...context, blocks: pairs(125, 126, 124, 125, 80) }), + ).toBe("slower"); }); - it("flags faster when a majority of blocks agree past the floor", () => { - const branch = { median: 70, min: 60, max: 85, p90: 85 }; - expect(timingVerdict(base, branch, { ...context, blocks: pairs(70, 65, 75, 110, 72) })).toBe( + it("flags faster when most blocks are clearly faster", () => { + expect(timingVerdict(unused, unused, { ...context, blocks: pairs(75, 74, 76, 75, 120) })).toBe( "faster", ); }); - it("falls back to the range check below three blocks", () => { - const branch = { median: 130, min: 115, max: 150, p90: 150 }; - expect(timingVerdict(base, branch, { ...context, blocks: pairs(60, 60) })).toBe("slower"); + it("reads mixed-sign blocks as within noise", () => { + expect(timingVerdict(unused, unused, { ...context, blocks: pairs(180, 70, 130, 75, 85) })).toBe( + "within noise", + ); }); -}); - -describe("timingVerdict with a wider build", () => { - const context = { comparable: true, samples: 10 }; - it("reads a slower median as noise when the base median sits inside the branch range", () => { - const base = { median: 225, min: 218, max: 240, p90: 238 }; - const branch = { median: 274, min: 218, max: 305, p90: 300 }; - expect(timingVerdict(base, branch, context)).toBe("within noise"); + it("reads block medians with wide spread as noise", () => { + const blocks = [ + [222, 276], + [224, 252], + [227, 225], + [233, 299], + [222, 291], + ].map(([base, branch]) => ({ base: base!, branch: branch! })); + expect(timingVerdict(unused, unused, { ...context, blocks })).toBe("within noise"); }); - it("reads a faster median as noise when the base median sits inside the branch range", () => { - const base = { median: 11211, min: 11172, max: 11224, p90: 11224 }; - const branch = { median: 10557, min: 10381, max: 11236, p90: 11236 }; - expect(timingVerdict(base, branch, context)).toBe("within noise"); + it("falls back to the run path below three blocks", () => { + const branch = timings(repeat(10, 130)); + expect(timingVerdict(unused, branch, { ...context, blocks: pairs(60, 60) })).toBe("slower"); }); });