Skip to content

perf(lanes): returning to Lanes keeps the lane's PR status on screen while it refreshes - #1731

Closed
arul28 wants to merge 1 commit into
mainfrom
perf/lanes-pr-cache
Closed

arul28 wants to merge 1 commit into
mainfrom
perf/lanes-pr-cache

Conversation

@arul28

@arul28 arul28 commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Why

Switching Work → Lanes settles late (376 ms warm on v1.2.109, ceiling 250 ms). The coordinator's IPC trace of one switch listed the selected lane's PR detail reads (pr.getStatus, pr.getChecks, pr.getFiles, pr.getReviews) and several lane git reads among the calls it waited on.

Findings from reading the code:

  • The lane detail's PR reads are already cached in the renderer. useLanePrDetail (components/lanes/overview/useLaneOverviewData.ts, prDetailCache, since feat(desktop): project sidebar shell and Lanes redesign #1304) shows the cached detail at once and skips the re-read while the cached read is under 60 s old. A throwaway hook test (not committed) confirmed it: park and unpark within 60 s makes 0 extra reads and shows the cached checks. After 61 s the cached detail still shows at once, one re-read runs, and a changed result replaces it. So a return within 60 s does not re-issue the four PR detail reads, and a return after 60 s shows the cached detail while they run. The tabs script's warm rounds are a few seconds apart, so they should not re-issue them either, but no real-app trace has confirmed that. Not changed here.
  • The lane list regroups on every return. LanesPage clears lanePrTags and laneGithubPrTags on every activation (LanesPage.tsx:838) and then re-reads them. classifyLaneState and laneNeedsYouReason read those tags to place each lane in a sidebar group: a merged PR makes a lane "done", failing checks, conflicts or requested changes make it "needs you". So each return regrouped lanes with PRs twice: blank, then filled. That is the on-return churn this PR removes.

What changed

apps/desktop/src/renderer/components/lanes/LanesPage.tsx only.

  1. PR tags survive re-activation for the same project. The reset now runs only when activeProjectRoot changes (lanePrTagsProjectRootRef). Activation still runs refreshLanePrTags() and refreshLaneGithubPrTags() exactly as before. Their results replace the kept rows when they arrive.
  2. The visible-PR refresh marks a PR as requested when its request is sent, not when the timer is queued. Needed by (1): a fresh tag list arriving inside the 260 ms debounce cancels the queued timer and re-selects. With the old order the PR stayed marked for 15 s with no request sent, so keeping the rows on return would have dropped that refresh. Side effect: a refresh that the old order dropped in this way is now sent. The per-PR 15 s throttle still applies. No new timers or polling.

Freshness: the activation reads, their timing, and their results are unchanged. Nothing arrives later. The only difference is that on return the rows show the last known PR state until the fresh read lands, instead of showing nothing.

Evidence

Metric (named before coding): Lanes tab warm settled time, the time from the switch click until the DOM stops changing, by ROUTES=/work,/lanes node scripts/perf/ui/interaction.mjs tabs 7 (add SCREENCAST=1 when the window is covered) on the installed app with real data. Secondary: warm first frame, long frames, DOM nodes, and IPC calls on open (docs/perf/macos-baseline.md, "Lanes: IPC on open").

Baseline (coordinator, installed v1.2.109, quiet machine): warm settled 376 ms (ceiling 250 ms), warm first frame 58 ms, 0 long frames, ~6460 DOM nodes.

After: to be measured by coordinator on the installed app with the dev build. No after numbers are claimed in this PR.

Checks run in the worktree (Node 22.13.1):

  • npm --prefix apps/desktop run typecheck: passes.
  • npx eslint src/renderer/components/lanes/LanesPage.tsx: 0 errors. Two warnings, both pre-existing and outside the diff (react-hooks/exhaustive-deps at lines 443 and 2197).
  • npx vitest run src/renderer/components/lanes/LanesPage.test.ts: 49/49 pass.
  • Throwaway useLanePrDetail test (deleted, not committed): 2/2 pass, as described under Why.

Not checked

  • Real-app run. Not run here: the installed app was not launched. The before/after settle, long-frame and IPC numbers are the coordinator's to take.
  • No automated test covers this change. LanesPage is a large App-level component. A focused test would need most of its dependencies mocked, so none is committed. The two behavior changes were verified by reading and by typecheck and lint only.
  • The lane list still regroups on return from the agents side. useLaneAgents (components/lanes/laneAgents.ts) clears its roster while the page is parked and re-reads it on return, and the same classifyLaneState reads agents. This PR does not touch that. It is probably the rest of the remaining churn; it is a separate concern (the chat roster).
  • Git status. LaneGitActionsPane already keeps its last state on re-activation (initialCachedGitState), and the list's git fields come from the store. Neither is changed here, so the title covers PR status only.
  • Windows and remote lanes. Renderer-only. Remote-machine lanes use allMachineLanes.prsByMachineId, which this change does not touch. Not verified on Windows or on a remote machine.
  • Perf table. docs/perf/macos-baseline.md is not updated. There is no measured after yet; the coordinator should record the new baseline with the commit once measured.

🤖 Generated with Claude Code

ADE   Open in ADE  ·  perf/lanes-pr-cache branch  ·  PR #1731


Note

Low Risk
Renderer-only caching and debounce timing in LanesPage; refresh paths and freshness behavior are unchanged aside from avoiding empty UI and fixing a queued-but-never-sent refresh edge case.

Overview
Returning to the Lanes tab no longer wipes PR tag state when you stay on the same project, so sidebar grouping (done / needs you / etc.) keeps showing the last known PR status until the existing activation refresh completes instead of flashing empty and regrouping twice.

LanesPage tracks the project with lanePrTagsProjectRootRef and only clears lanePrTags / laneGithubPrTags when activeProjectRoot changes; activation still runs the same refreshLanePrTags and refreshLaneGithubPrTags calls.

The debounced visible-PR background refresh now records each PR in laneVisiblePrRefreshRequestedAtRef when window.ade.prs.refresh is actually sent, not when the 260 ms timer is scheduled, so a tag update that cancels and reschedules the debounce does not leave PRs throttled for 15 s without a request.

Reviewed by Cursor Bugbot for commit d122b0e. Configure here.

…while it refreshes

Lanes cleared its PR tags on every activation and read them again. The lane
list groups lanes from those tags (a merged PR makes a lane "done", failing
checks or conflicts make it "needs you"), so each return regrouped the sidebar
twice: blank, then filled.

Keep the tags while the same project is shown and clear them only on a project
switch. The activation reads still run, and their results replace the kept rows
when they arrive.

The visible-PR refresh now records a PR as requested when its request is sent,
not when the timer is queued. A fresh tag list arriving inside the 260 ms
debounce cancels the queued timer and re-selects. Marking at queue time left
the PR marked with no request sent, so its refresh was dropped for 15 s.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 17, 2026

Copy link
Copy Markdown

Deployment failed for project ade with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/arul28s-projects?upgradeToPro=build-rate-limit

@cursor

cursor Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 94ff737a-2dac-445f-884f-ecae4fc3bf1e)

@coderabbitai

coderabbitai Bot commented Oct 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 31 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: arul28/ADE/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 43a3bb4f-5f76-4979-a05f-24e015abe70d
📥 Commits

Reviewing files that changed from the base of the PR and between 83786e3 and d122b0e.

📒 Files selected for processing: 1
  • apps/desktop/src/renderer/components/lanes/LanesPage.tsx
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@arul28

arul28 commented Oct 17, 2026

Copy link
Copy Markdown
Owner Author

Parking this per the perf merge gate: no proven win.

Coordinator A/B in the dev app (same session, HMR-swapping LanesPage.tsx between this branch and main, SCREENCAST=1 ROUTES=/work,/lanes node scripts/perf/ui/interaction.mjs tabs 7, quiet machine via quiet.sh), Lanes warm settled:

run this PR main
1 177 ms 386 ms
2 457 ms 209 ms

The ranges overlap completely, so the difference is inside run-to-run noise. This branch also renders more nodes on Lanes (3,950 vs ~2,210), because the kept PR tags stay on screen.

Lead for a next attempt (from the implementer): useLaneAgents also clears while the page is parked and classifyLaneState reads it, which may be the rest of the regroup on return. Real-app trace showed the Lanes settle on v1.2.109 waiting on GitHub-backed pr.getStatus (~1.07 s) / pr.getChecks (~0.8 s) on a return more than 60 s after the last read.

Closing without merge; the branch stays for reference.

🤖 Generated with Claude Code

@arul28 arul28 closed this Oct 17, 2026
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