Agentic UI: Stop the About card's disk measurement from thrashing on site switches - #4627
Open
shaunandrews wants to merge 1 commit into
Open
Agentic UI: Stop the About card's disk measurement from thrashing on site switches#4627shaunandrews wants to merge 1 commit into
shaunandrews wants to merge 1 commit into
Conversation
…site switches Resolve site paths from cli.json instead of forking the CLI, wait for a site to settle before measuring it, and cancel a measurement when the user leaves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
📊 Performance Test ResultsComparing 16be217 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) |
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 investigated the report, measured the costs below, and wrote the fix and tests. Numbers in "Proposed Changes" are measured on a real machine (15 sites, 3,700–12,400 files each), not estimates.
Proposed Changes
Switching between a few sites quickly made the whole app crawl. The About card's Disk row measured the site's folder from scratch on every switch, moving on didn't stop the measurement already running, and nothing waited to see whether the user actually meant to land on that site — so flicking through eight sites in ten seconds left eight full measurements in flight at once.
Three things caused it, all fixed here:
GET /sites/:id/storage(and/wp-version, which the same card requests) calledlistSitespurely to map a site id to its path. That fork measured 1.9s wall and 1.2s CPU; six concurrent ones took 2.5s wall and ~7s CPU. Both routes now read the path straight out ofcli.json— 0.1ms, verified against all 15 real sites on disk. Routes that need live fields or must observe a write the CLI just made still go throughlistSites.AbortSignal, so leaving a site left its walk running — on Desktop, inside the main process. The signal is now honored end to end: the walk itself checks it, the browser ties it to the HTTP request, and Desktop cancels through a companion IPC call sinceipcRenderer.invokehas no cancellation of its own.No visual change — the Disk row looks and reads exactly as before.
Two related things this deliberately leaves alone, noted in STU-2305 for follow-up: the measurement still isn't cached or deduped server-side, and the thumbnail/wp-version queries still inherit
staleTime: 0so they refetch on every mount and window focus.Testing Instructions
Browser UI (
npm run cli:build:ui && node apps/cli/dist/cli/main.mjs ui --no-open, then http://localhost:8081):/api/sites/does-not-exist/storageand confirm it still 404s.Desktop: same walkthrough in the app. Rapid site switching should no longer bog down the UI.
Unit tests:
npm test -- apps/cli/tests/local-server.test.ts packages/common/sites apps/ui/src/data/queries/use-site-storage-usage.test.tsxFull suite run before/after: identical failure sets (39 pre-existing failures in this worktree, all from missing optional deps —
@wp-playground/blueprints,wpcom/types,@types/archiver), zero introduced.Pre-merge Checklist