Shard the browser suite across four runners - #225
Merged
Conversation
The per-step timings from the first push run are what make this the lever worth pulling: 708s of the job's 774 were inside `Run Playwright specs`, and everything else - checkout, vp install, the cold-cache browser download, both dev servers - came to 56s. The part that shards is essentially all of it, so each added runner pays about a minute to take a quarter off the rest. Sharding rather than raising `workers`, which is the other obvious knob and the wrong one. playwright.config.ts pins `workers: 1` because the specs share ports 8080/8081 and a single editor instance, and that conflict is within a machine. Giving each shard its own runner, and so its own pair of dev servers, sidesteps it rather than solving it. `fail-fast: false`, because a red shard should not cancel the other three. The reason to run the suite is to learn what broke, and three cancelled shards answer that worse than three finished ones. `deploy` still says `needs: [checks, e2e]` and needs no change: a matrix job is not satisfied until every leg succeeds. Renaming the job to "Playwright n/4" pins nothing loose - checked, the repo has no branch protection and its single ruleset is a disabled force-push rule, so no required check is referenced by name. No wall-clock figure is claimed here yet. The last time this job's cost went into a comment it was extrapolated from a laptop and wrong by 2.6x (#224); this one gets written down after the run measures it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dbza77QEZnkqt1mGssY71
The four legs on this job's own first run: 201/187/192/213s wall clock, of which 160/144/144/153 were specs. So 774s becomes 213 - the slowest leg is the wall clock, not the mean - a 3.6x speedup for 561s saved, and the deploy behind it waits three and a half minutes rather than thirteen. The prediction in the previous commit was wrong in a way worth keeping. It said to expect lopsided shards, reasoning that Playwright splits by test count while the corpus-wide specs dwarf the rest. Measured, the legs land within 11% of each other. The reasoning was sound and the answer was not, which is the usual argument for measuring. Also fixes CLAUDE.md's deploy-cost sentence, which #224 missed while correcting six others. It has now carried a wrong number twice - "five minutes" guessed from a laptop, then thirteen once #224 corrected everything around it - so it now says to believe ci.yml and re-measure if the two ever disagree. A sentence that has been wrong twice is worth annotating rather than just correcting. Swept the whole tree for stale minute-claims afterwards rather than trusting one grep pattern, which is how the miss happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dbza77QEZnkqt1mGssY71
The second run measured 214/168/220/225s where the first measured 201/187/192/213. Both are the same suite on the same code, so the slowest leg is 213-225s rather than 213, and quoting one sample would have read as precision the number does not have. Nothing about the change moves: still roughly 3.5x, still under four minutes for the deploy to wait where it used to be thirteen. This is the third pass over these same sentences today, after guessing five and measuring thirteen. Two samples is what it took to stop writing a figure that the next run contradicts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dbza77QEZnkqt1mGssY71
The third run's slowest leg was 250s, outside the 213-225 range the previous commit had just recorded from two samples. Same suite, same code, third answer. So the figure is not 213, and it is not 213-225 either. It is roughly three to four minutes on a shared runner, and the spread is now written down alongside all three samples with an instruction not to tighten it back into one number. A slow run here is also not a regression without a second sample, which is worth saying before somebody chases one. This sentence has now been wrong four times in a day: five minutes guessed from a laptop, thirteen measured unsharded, 213 from one sharded run, 213-225 from two. Each correction was itself quoted too precisely. The pattern is not that the earlier numbers were careless - each was the honest reading of the evidence in hand - it is that a single sample of a shared-runner timing reads as a fact and is not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dbza77QEZnkqt1mGssY71
This was referenced Aug 17, 2026
wormeyman
added a commit
that referenced
this pull request
Aug 17, 2026
A vitest text scan over tests/*.spec.ts for `Control+A` and its four siblings. On macOS `Control+A` is the emacs "beginning of line" binding, not select-all, so nothing is selected, the typed text lands beside the old text, and the assertion reads "New textOld text". That is #197, fixed in 402fbe3. It reads source rather than driving a browser because the bug **passes on Linux**, and every runner this project has is ubuntu-latest - `vp test` and all four `e2e` shards. #223 and #225 put Playwright in CI, which does not help here: a sharded browser suite gating every PR will go green on this forever. The rule is not "never write Control". A chord reaching the app through actions.ts stays Control, where ModifierKey is Control | Shift | Alt and no Meta binding exists; a chord reaching a focused DOM <input> wants ControlOrMeta. A regex cannot separate them, so it flags both and the ALLOWLIST carries the distinction in writing - one entry today. Mutation-checked against the live instance: dropping PR #222's blueprint-grid-position.spec.ts into tests/ fails the guard naming line 97, and removing it goes green again. The knowledge already existed in prose. After 402fbe3 there is a five-line comment at display-panel-editor.spec.ts:159 explaining the whole trap, one line above its own corrected call, and a contributor then wrote `Control+A` in a different file. A comment at the scene of the last occurrence does not reach the next one. Every figure in the new file's header is measured rather than recalled, which caught two: the display panel comment is five lines and not three, and four specs hold `keyboard.down('Control')` for the ctrl-drag gesture and not five. Both came from #208's own write-up. Part 1 of #208. Deliberately not `Closes`: that issue's part 2, the display panel editor's uncovered surface (the alt-mode checkbox, the icon picker and the connected branch), is untouched here and is real work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg2oe7JgL3xEzSoN5emTsd
wormeyman
added a commit
that referenced
this pull request
Aug 17, 2026
A vitest text scan over tests/*.spec.ts for `Control+A` and its four siblings. On macOS `Control+A` is the emacs "beginning of line" binding, not select-all, so nothing is selected, the typed text lands beside the old text, and the assertion reads "New textOld text". That is #197, fixed in 402fbe3. It reads source rather than driving a browser because the bug **passes on Linux**, and every runner this project has is ubuntu-latest - `vp test` and all four `e2e` shards. #223 and #225 put Playwright in CI, which does not help here: a sharded browser suite gating every PR will go green on this forever. The rule is not "never write Control". A chord reaching the app through actions.ts stays Control, where ModifierKey is Control | Shift | Alt and no Meta binding exists; a chord reaching a focused DOM <input> wants ControlOrMeta. A regex cannot separate them, so it flags both and the ALLOWLIST carries the distinction in writing - one entry today. Mutation-checked against the live instance: dropping PR #222's blueprint-grid-position.spec.ts into tests/ fails the guard naming line 97, and removing it goes green again. The knowledge already existed in prose. After 402fbe3 there is a five-line comment at display-panel-editor.spec.ts:159 explaining the whole trap, one line above its own corrected call, and a contributor then wrote `Control+A` in a different file. A comment at the scene of the last occurrence does not reach the next one. Every figure in the new file's header is measured rather than recalled, which caught two: the display panel comment is five lines and not three, and four specs hold `keyboard.down('Control')` for the ctrl-drag gesture and not five. Both came from #208's own write-up. Part 1 of #208. Deliberately not `Closes`: that issue's part 2, the display panel editor's uncovered surface (the alt-mode checkbox, the icon picker and the connected branch), is untouched here and is real work. Claude-Session: https://claude.ai/code/session_01Mg2oe7JgL3xEzSoN5emTsd Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Follow-up to #223 and #224. The deploy now waits on the browser suite, so the suite's wall clock is the deploy's wall clock.
Why sharding is the lever
Per-step timings from the first push run:
708 of 774 seconds are inside the specs. The part that shards is essentially all of it, so each added runner pays roughly a minute of setup to take a quarter off the rest.
Why not just raise
workersThat's the other obvious knob and it's the wrong one.
playwright.config.tspinsworkers: 1because the specs share ports 8080/8081 and a single editor instance. That conflict is within a machine. Giving each shard its own runner, and so its own pair of dev servers, sidesteps it rather than solving it.Details worth flagging
fail-fast: false— a red shard shouldn't cancel the other three. The reason to run the suite is to learn what broke, and three cancelled shards answer that worse than three finished ones.deployneeds no change. It still saysneeds: [checks, e2e]; a matrix job isn't satisfied until every leg succeeds.Playwright n/4pins nothing loose. Checked: the repo has no branch protection, and its single ruleset is a disabled force-push rule, so no required check is referenced by name.No wall-clock claim yet
Deliberately. The last time this job's cost went into a comment it was extrapolated from a laptop and wrong by 2.6x (#224). The number goes in after this PR's own run measures it, and before merge.
Verified so far:
vp check .clean; parsed YAML confirmsmatrix.shard [1,2,3,4],fail-fast: false, the--shardflag threaded fromstrategy.job-total, per-shard artifact names, anddeploy needs: ['checks', 'e2e']unchanged.🤖 Generated with Claude Code
https://claude.ai/code/session_017Dbza77QEZnkqt1mGssY71