Repository navigation
Studio: Show the site a start or stop reports, instead of waiting for its process to exit - #5524
Merged
Merged
Conversation
… lets the process exit
… its process to exit
Collaborator
📊 Performance Test ResultsComparing e115743 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
3 tasks
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
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.
Related issues
How AI was used in this PR
Claude (via the Pi coding agent in a Roadie session) traced the start/stop timing, wrote the change and the test, and measured the CLI message ordering below. I reviewed the diff and the measurements.
Proposed Changes
Starting or stopping a site from the app resolves when the
site start/site stopprocess exits. After finishing, both commands send a Tracks event, so the status button keeps its spinner while the request completes.Since #5522, the start/stop could resolve on the CLI's
donemessage instead, but that loses a race. The site stays "transitioning" until the event releasing its start/stop operation arrives, and that event reaches the app in the same millisecond asdone(8 of 8 runs). Whendonewins, the app sees the operation still held and refetches the whole site list. In a CI run with only that change,siteStartupwent from 1,594 to 1,868 ms (build 24349).This PR removes the race instead of waiting it out:
site startandsite stopreport the site after the operation is released, with its running state.startSite/stopSiteresolve with that site when the command reportsdone, and the app writes it into the cached list the same way it applies a site event.Measured with a production CLI build forked over IPC:
running: true, no operationrunning: false, no operationdoneto exit, telemetry on (from #5522)That tail is the time the app no longer waits after a start or stop. It depends on the user's connection. CI skips Tracks, so CI's performance metrics should stay flat for
siteStartup.Testing Instructions
npm test -- apps/ui/src/data/queries/use-sites.test.tsxPre-merge Checklist