Skip to content

perf(planning): open the journey suite's browser with settle 0 - #11943

Merged
kyle-sexton merged 1 commit into
mainfrom
perf/8757-journey-phases
Nov 1, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
perf/8757-journey-phases

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #8757

Summary

Problem: surface-journey.test.sh runs over its 120 s goal on main. After #11847 the median of four whole-suite runs was 126.5 s (127, 126, 101 and 129 s); #11847's 6-10 s local saving did not show above CI's run-to-run swing. This PR removes a cost larger than that noise: playwright-cli's after-script settle wait, paid once for each of the journey's 20 phase scripts.

Fix

  • The journey suite's browser session opens with PLAYWRIGHT_MCP_TIMEOUT_SETTLE=0. After every run-code call playwright-cli waits its settle time (500 ms by default), waits for the script's page requests, and waits again when there were any: about 1 s per call. The journey's phase scripts already wait for what they check, so the extra wait buys nothing. The setting takes effect only at open, and only for --part journey; the other parts keep the default.
  • No phases were merged: of the 19 phase boundaries, the 16 with shell writes between them must keep those writes in place, and merging the 3 without would save about 0.75 s.

Verification

  • surface-journey.test.sh locally, 9 runs, all PASS=256 FAIL=0: wall time median 69.9 s before (68.5, 70.3, 69.5, 72.8 s) and 53.4 s after (52.1, 51.4, 60.0, 64.3, 53.4 s), about 16.5 s less. Two slower after-runs coincided with slow shell steps (host load); an interleaved before/after pair gave 72.8 s against 53.4 s.
  • CI installs the same @playwright/cli 0.1.22 (package.json:13).
  • Left out: the CI median. perf(test): cut the local affected-test gate's shell suites to a budget #8757 closes only when main's median over 8-10 whole-suite runs is under 120 s, so this PR refers to it.

Related

  • perf(planning): cut the journey suite's shell and page waits #11847, the earlier shell and wait cut.
  • Risk named for review: a shell write now follows a phase's last page action with no 0.5-1 s gap. Phases 1, 4 and 17 end on an action with no wait after it; none flaked in 5 local runs. If one flakes on CI, the fix is a short settle() at the end of that phase.

🤖 Generated with Claude Code

Each journey phase is its own playwright-cli run-code call. After the
script returns, run-code waits its settle time (500 ms by default), then
waits for the script's requests to finish and settles again: about 1 s
per call (waitForCompletion in playwright-core's coreBundle.js). A
trivial run-code measured 0.72 s at the default and 0.25 s with
PLAYWRIGHT_MCP_TIMEOUT_SETTLE=0. The setting is read only when the
session opens, so the suite sets it at pw open for the journey part
alone; the other parts' groups keep the default.

Merging phases into fewer calls was mapped and left out: only 9|10,
11|12 and 16|17 have no shell step between them, and each merge would
now save about 0.25 s.

Local wall times, surface-journey.test.sh, PASS=256 FAIL=0 on every run:
- before: 68.5, 70.3, 69.5, 72.8 s (sum of per-phase lines 60, 62, 60, 61 s)
- after:  52.1, 51.4, 60.0, 64.3, 53.4 s (per-phase sums 44, 44, 49, 55, 45 s)
Median 69.9 s to 53.4 s. The 60.0 and 64.3 s runs came with slower
shell steps too (apply 2.8 s against 1.7 to 2.0 s), host load.

Refs: #8757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 1, 2026 14:32
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T14:34:17.181929Z 24de2d1 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 24de2d163d1b0f9b14e21376d61fad60b87299c6: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #11943

No security issues found.

I read every hunk of both changed files:

  • plugins/planning/surface/surface-checks.sh: The journey suite's local Playwright session now opens with PLAYWRIGHT_MCP_TIMEOUT_SETTLE=0. The variable is set only on the pw open command, and only when $part equals journey. That changes a timing setting in a local test harness. It adds no input path, no credentials and no injection point. The value is a literal 0, and the [[ ... ]] string comparison against the literal journey treats $part as data, never runs it.
  • .changes/planning/perf-8757-journey-settle-0-7d2a5e91.md: A changelog fragment with bump: none. It is prose only.

Neither file matches a trusted-path glob. The change touches no trust control, kill switch, token scope, merge hold, plugin manifest or standing instruction surface, so it removes no guardrail.

The PR context contains no embedded instructions aimed at the review lane.

Spend: none. The change shortens test runtime and adds no runs.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 24de2d163d1b0f9b14e21376d61fad60b87299c6: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

I read every hunk of both changed files: plugins/planning/surface/surface-checks.sh and .changes/planning/perf-8757-journey-settle-0-7d2a5e91.md.

What changed. For --part journey only, the shared browser session now opens with PLAYWRIGHT_MCP_TIMEOUT_SETTLE=0. Every other part, all included, still opens with playwright-cli's default settle time.

Checks

  • Does the variable reach the browser? Yes. pw is a shell function that runs playwright-cli in a subshell (surface-checks.sh#L131). In bash, a variable set in front of a function call is exported for that call, so playwright-cli open sees it. The setting doesn't leak into later pw calls, which the comment says is fine because the setting is read only at open.
  • Is $part set by that point? Yes. It is set from the argument parsing at L29-L32, and journey is one of the parts.
  • --part all: the journey groups keep the default settle time there. That only makes them slower, not wrong, and it matches the PR description.
  • Changelog fragment: bump: none fits a test-harness-only change.
  • Path-scoped rules: no .claude/rules/ glob matches either changed file. No plugin manifest is changed.

Things I could not verify (author-claimed)

  • I can't run anything in this lane, so I have not confirmed that PLAYWRIGHT_MCP_TIMEOUT_SETTLE is an environment variable @playwright/cli 0.1.22 actually reads. If it isn't, the change does nothing but is still safe.
  • I also have not confirmed the local timings (69.9 s median before, 53.4 s after) or the claim that 5 local runs had no flakes.
  • The PR names its own risk: phases 1, 4 and 17 end on a page action with no wait after it, and a shell write now follows straight away. That's worth watching on CI. The PR's fallback is a short settle() at the end of any phase that flakes.

Closing issue: the PR says Refs: #8757 and does not close it, so there are no acceptance criteria to check.

Spend: none beyond CI minutes, and this change should reduce them.

@kyle-sexton

Copy link
Copy Markdown
Member Author

Lane 5 arm request for the hub. Hub sessions exit before a lane can reply over the socket, so I'm posting it here. Please arm at 24de2d163d1b0f9b14e21376d61fad60b87299c6 with arm-when-verified.sh 11943 24de2d163d1b0f9b14e21376d61fad60b87299c6.

Gate state:

  • ready for review, no labels
  • ci-status green
  • claude-review, claude-security-review and Codex report 0 findings on this head, with no review threads
  • pr-verify is pending; the script waits for it

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit bbdfcce Nov 1, 2026
71 of 72 checks passed
@kyle-sexton
kyle-sexton deleted the perf/8757-journey-phases branch November 1, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant