[ci] Overhaul performance benchmarks: focused metrics + sticky PR comment#2820
Merged
Conversation
Replaces the sprawling vitest-bench suite with three focused scenarios measuring TTFS, STSO, WO and SO (avg/p50/p90/p99), run many times against a Vercel deployment. Results are posted to a single sticky PR comment that keeps previous runs in a collapsed history section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
🦋 Changeset detectedLatest commit: 1ac3289 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📊 Workflow Benchmarkscommit Backend:
📜 Previous results (5)ab54b48Wed, 08 Jul 2026 21:18:02 GMT · run logs
c2b5794Wed, 08 Jul 2026 20:55:35 GMT · run logs
c2b5794Wed, 08 Jul 2026 20:35:13 GMT · run logs
874c3a1Wed, 08 Jul 2026 19:06:06 GMT · run logs
874c3a1Wed, 08 Jul 2026 18:52:47 GMT · run logs
Metrics — TTFS: time to first step body execution · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (time outside step bodies, client start → last step body exit) · SL: stream latency (first chunk write → visible to the reader) Scenarios — stream: one step that streams chunks back to the client; no hooks, so the run stays in turbo mode · hook + stream: registers a hook before the same streaming step, which exits turbo mode · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges 🟢/🔴 mark percentiles within/above target. Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 TTFS/WO compare client vs deployment clocks and SL compares the step runner’s clock vs the client’s (NTP-synced in CI). WO ends at the last step body exit, the closest observable proxy for the final step-completion request. |
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
TooTallNate
approved these changes
Jul 8, 2026
TooTallNate
left a comment
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Reviewed the harness, the workflows, the CI plumbing, and the renderer. Approving — this is a big improvement, and it directly fixes the operational problem I've been triaging all week (benchmark lanes hanging for 60–90 minutes as unbounded cancellations across main and every PR).
What I verified:
- Harness design is sound. Sequential (non-concurrent) iterations are the right call for latency percentiles; the bounded failure tolerance (
MAX_FAILURE_RATIO) plus per-runwithTimeout(withunref, and the documented decision to notreader.cancel()on timeout — consistent with the known cancel-can-hang behavior) means one stuck run can no longer eat the job. Clock-skew caveats for TTFS/WO/SO are documented honestly, and STSO is skew-free by construction (both timestamps from the deployment clock). - Metric derivations check out: WO =
(lastStep.end - clientStart) - Σ(step durations)with body-entry/exit timestamps recorded in the workflow, STSO from consecutive body boundaries, SO fromwrittenAtembedded in chunks with the reader attached before the step runs. - CI structure: every phase has
timeout-minutes(5/30/60/10) — no more unbounded hangs;fail-fast: false; concurrency group cancels superseded PR runs but not main; third-party actions are SHA-pinned; results artifact uploads onalways()so partial data survives a failed scenario;GITHUB_SHAcorrectly reports the PR head SHA rather than the synthetic merge commit. - Cleanup is complete: no stale references to
bench.bench.ts,aggregate-benchmarks, or therender-benchmarksaction anywhere;vitest.config.tsbenchmark block and root scripts updated; the workbench symlinks for97_bench.tsare intact across apps. - Renderer: 8/8
node --testtests pass locally, and lint.yml's existing.github/scripts/**/*.test.jsglob picks them up as claimed. The base64-embedded history block surviving re-renders is a neat, self-contained persistence trick. - Empty changeset is correct for a CI/test-infra-only change.
Two non-blocking notes:
- With 30 stream iterations, nearest-rank p99 equals the max sample — fine (and standard), just worth remembering when reading the comment table that p99 is effectively "worst observed" until iteration counts grow.
bench:localwas dropped from root scripts;DEPLOYMENT_URL=http://localhost:3000 APP_NAME=nextjs-turbopack pnpm benchcovers it, but if local runs are common during perf work it may be worth keeping the alias. (Note the SO scenario's local-world limitation is already documented in the harness header.)
…ules The download-artifacts + pnpm install pattern hangs in workbench postinstall scripts (sveltekit prepare stalled for the full job timeout). Mirror the e2e-vercel jobs: fresh install via setup-workflow-dev + turbo build of the CLI dependency graph, which hits the remote cache. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions - Rename SO -> SL (stream latency) - Metric definitions move from the table rows to the comment footer - Sequential scenario: 1020 steps, 1 iteration, STSO reported per step-index range (1-20, 20-120, 120-1020); WO no longer reported for it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single long sequential run can fail transiently (e.g. replay divergence under a large event log — the known corrupted-event-log flake family), which previously zeroed out the whole scenario at 1 iteration. Scenarios now backfill failed iterations with spare attempts and only fail when the budget can't produce the requested sample count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Scenario names shortened; explanations move to a footer legend emitted by the runner in the results JSON - ms moves into the table column headers - p50 replaced with p75 - p75/p90/p99 cells get 🟢/🔴 marks against per-metric targets (TTFS 200/300/600, STSO windows 20/30/60, 30/45/90, 40/60/120, SL 50/60/125); STSO windows are now steps 1-20, 101-120, 1001-1020 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Backport to This is usually an infrastructure problem (e.g. the configured AI model could not be found, an AI Gateway error, or an opencode crash) rather than a merge conflict. Check the job logs linked above for details. Once the underlying issue is fixed, re-run the Backport to stable workflow manually via |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the sprawling vitest-bench benchmark suite with three focused scenarios that measure the runtime's core latency metrics, each reported as avg / p50 / p90 / p99 in a single sticky PR comment. Re-runs update the same comment; older results collapse into a
<details>history section (the data is embedded in the comment body as a base64 block, so history survives re-renders).Metrics
start()timestamp (run_created request) → first step body executionsteps[i].start - steps[i-1].end)(lastStep.end - clientStart) - Σ(step durations)run.getReadable()readerScenarios (
workbench/example/workflows/97_bench.ts)benchStreamWorkflow— 1 streaming step, no hooks → runs in turbo mode → TTFS/SO/WO (turbo)benchSequentialStepsWorkflow— 100 trivial sequential steps → STSO (99 samples/run) + WObenchHookStreamWorkflow— fire-and-forgetcreateHook()before the same streaming step → exits turbo → TTFS/SO/WO (non-turbo)Steps record
Date.now()at body entry/exit and the workflow returns the timings; stream chunks embed their write timestamp. Iteration counts are env-tunable (BENCH_*, defaults: 30 stream iterations, 5×100 sequential).CI
benchmarks.ymlnow runs a single Vercel benchmark job (nextjs-turbopack) driven by a matrix that can later grow postgres/local entries — the runner already picks its backend from the same env vars as the e2e tests.render-benchmark-comment.mjsrenders the running/completed/failed comment states, with unit tests picked up by lint.yml'snode --test.bench.bench.ts,aggregate-benchmarks.js, therender-benchmarksaction,benchmark-community-world.yml, and the baseline/GH-Pages publishing plumbing (nothing consumes the published benchmark JSON).Notes
nextjs-turbopackdev server (all 3 scenarios + results JSON + comment rendering).Verification
🤖 Generated with Claude Code