perf(client): equal query refetches no longer re-render PR rows

[juliusmarminge]

Problem

The render audit looked at the thread details panel of a thread with 15 linked PRs. While idle, each row's checks query refetches every 45-60s. Each refetch publishes a value with new identities and the same content, so every row, its controls, and its tooltip re-render even when no check changed. That came to about 30 renders per minute while idle (lane 4: 30 renders, top cause ThreadDetailsPrRow state#6, value.checks[0..n] identity only).

Fix

• createEnvironmentQueryAtomFamily (client-runtime) passes every resolved and emitted value through the existing reuseUnchanged helper, against the previous success. An equal refetch keeps the previous value reference, and so does each unchanged nested object or array. A genuine change still publishes new references, and only for the parts that changed. This applies to every environment query, including the persisted ones (the reused value is also what gets saved).
• ThreadDetailsPrRow compares timestamps into a boolean before merging detail and checks. The compiled output now keys the merged detail on [checksAreNewer, checksQuery.data, detailQuery.data] instead of the raw timestamps, which move on every refetch. With the data stable, detail stays stable, and the downstream compiler cache slots ($[n] !== detail, detail?.checks, the trailing actions and the checks tooltip) bail out.

The AsyncResult wrapper still changes on every refetch, on purpose. SWR staleness reads its timestamp, and the waiting flag drives the spinner and the refresh gating, so those transitions still publish.

What still renders

Each row component itself still renders on the waiting-on and waiting-off transitions of a refresh. Its children with unchanged props no longer do.

Callers that key effects on query.data identity (PullRequestDetailPanel snapshot save, CodexSetupSection handoff import) now fire less often. Both effects are idempotent or guarded.

Tests

The new runtime.test.ts case checks that an equal refetch returns the same reference and a changed refetch returns a new one. It fails without the fix.

After:

Update

The in-app re-measure showed no change: ThreadDetailsPrRow still rendered 20 times in 120 s idle. Keeping data stable was not enough, because every refetch publishes a new AsyncResult (new timestamp, waiting on and off), and the row read the whole result through useEnvironmentQuery.

The query atoms are unchanged beyond the first commit: SWR still reads the timestamp, and existing useEnvironmentQuery callers still see isPending. Readers that need less now subscribe to less:

• selectEnvironmentQueryData (client-runtime) selects a result's data alone. With the reuseUnchanged sharing, an equal refetch leaves it identical, so a subscriber to it is not notified.
• apps/web/src/state/query.ts adds useEnvironmentQuerySelector(atom, select), useEnvironmentQueryIsNewer(query, than) (a derived boolean atom comparing the two timestamps), and useEnvironmentQueryRefresh(atom), which returns refresh plus an isPending() read from the registry at call time.
• ThreadDetailsPrRow reads detail data, checks data, "checks are newer", and "checks settled empty" through those. It checks isPending() when its live refresh fires instead of subscribing to it. A poll that returns equal checks no longer re-renders the row. A changed poll, or detail being newer than checks, still does.

Compiled output: detail is keyed on [checksAreNewer, checksData, detailData], all three stable across an equal poll. Both files compile with no React Compiler diagnostics.

Test: runtime.test.ts "does not notify a data-only reader when a refetch returns equal content" subscribes to Atom.map(query, selectEnvironmentQueryData) and checks that an equal refetch notifies the result subscriber but not the data subscriber, and that a changed refetch notifies both. It fails without the reuseUnchanged sharing.

After: still to be re-measured in the app.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The optimization changes reference-identity and subscription behavior in the shared environment-query runtime, affecting many existing production consumers beyond PR rows. The targeted tests cover the new sharing behavior, but the broader effects and memoization implications should be checked across those consumers.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Team
• Run ID: c077980b-458c-4af4-b1e9-3b386db40160

📥 Commits

Reviewing files that changed from the base of the PR and between 8e3fced and 4f864cf.

📒 Files selected for processing: 5

• apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx
• apps/web/src/components/chat/ThreadDetailsPrRow.tsx
• apps/web/src/state/query.ts
• packages/client-runtime/src/state/runtime.test.ts
• packages/client-runtime/src/state/runtime.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

📝 Walkthrough

The environment query runtime now reuses prior successful values when refetched values are unchanged. New query hooks select data, compare update times, and expose refresh state. The PR row uses these hooks to select details and checks and control refreshes.

Changes: Environment Query and PR Row

Layer / File(s)	Summary
Reuse unchanged query values packages/client-runtime/src/state/runtime.ts, packages/client-runtime/src/state/runtime.test.ts	Environment query emissions and settled results reuse prior successful value references when content is unchanged, with or without persistence. Tests cover equal-content refetches, subscriber notifications, and changed content.
Select, compare, and refresh queries apps/web/src/state/query.ts	The query hooks select result values, compare update timestamps, and provide refresh and pending-state callbacks. The timestamp helper supplies dataUpdatedAt.
Select PR details and refresh checks apps/web/src/components/chat/ThreadDetailsPrRow.tsx, apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx	The PR row selects detail and check data, uses freshness to choose newer checks, and skips refreshes while queries are pending. It stops refreshing checks after a successful null result. The browser test mocks the selector and query state.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: t3dotgg, bil0000

Merge Risk: ⚪ Minimal · up to 4f864

No actionable merge-blocking regression was established; the change is mergeable after normal checks.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description clearly explains the problem, implementation, affected behavior, and tests. However, it omits the required Scope and approval section and does not provide completed verification result…	Add the required Scope and approval section with issue or maintainer-approval details, rename or expand the testing section as Verification, report observed test and performance results, include before/after UI screenshots where applicable,…

Passed checks: 3 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the primary change: preventing equal query refetches from re-rendering PR rows.

Full details: Description check

Explanation

The description clearly explains the problem, implementation, affected behavior, and tests. However, it omits the required Scope and approval section and does not provide completed verification results or the required UI screenshots. The reported measurements remain pending.

Resolution

Add the required Scope and approval section with issue or maintainer-approval details, rename or expand the testing section as Verification, report observed test and performance results, include before/after UI screenshots where applicable, and identify the model and harness used for the agent-generated work.

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[juliusmarminge]

Re-measured in the app (seeded data, same scenario, median of 3):

Measured against main at 489d08a, with this PR's commit cherry-picked onto that same commit (it applies cleanly, and still merges cleanly with today's main). The server ran without --watch, and no window had a reconnect. Thread "Take Over PR 6315" with 5 linked open PRs, thread details panel open with "Show 4 more" expanded, idle for 2 minutes per window (it spans checks polls at 45/60 s).

scenario	main	this PR
idle 120 s, total renders	568 (444 / 568 / 1660)	666 (595 / 666 / 781)
idle 120 s, ThreadDetailsPrRow renders	20 (12 / 20 / 35)	20 (20 / 20 / 15)

No improvement: the PR rows still re-render on every checks refetch. A hook-slot diff on ThreadDetailsPrRow with this PR shows slot #6 (the checks query AsyncResult) changing as #6.waiting=false #6.timestamp=…, four rows per poll. reuseUnchanged keeps data stable, but get.setSelf(AsyncResult.success(...)) still publishes a new AsyncResult with a new timestamp and a waiting flip. useEnvironmentQuery subscribes to the whole result (dataUpdatedAt, isPending), so the row re-renders anyway, and so do its tooltips (TooltipTrigger props:render,children ~110, ThreadDetailsControl handlers). The spread for detail on line 123 also builds a new object on every render. To stop this, skip setSelf when the reused value is identical (and leave waiting alone for background refetches), or have the row read only data through a selector.

The window-to-window spread (main #2 = 1660) comes from providerStatuses pushes landing in the window. Those were present on both sides, and are not this PR.

Behaviour check: rows and their states are unchanged ("Ready" on #19274/#19260/#19261/#19270, "Resolve" on #19259), and the checks keep refreshing.

[juliusmarminge]

Re-measured after the fix, in the app (seeded data, same scenario as round 2, median of 3):

This PR's two commits are cherry-picked onto main at 2fae0f9841 (they apply cleanly). The server ran without --watch, and no window had a reconnect. Thread "Take Over PR 6315" with 5 linked PRs, thread details open with "Show 4 more" expanded, idle for 120 s per window, which spans several checks polls.

scenario	main	this PR
idle 120 s, ThreadDetailsPrRow renders	20 (12 / 20 / 30)	0 (0 / 0 / 10)
idle 120 s, total renders	496 (378 / 496 / 1270)	259 (202 / 259 / 976)
idle 120 s, commits	22	4

This fixes the round-2 problem. On main, every poll re-renders each row through ThreadDetailsPrRow state#6 (the checks query result, with a new timestamp and waiting flip). With this PR, the checks polls cause no row renders at all. The 10 in run 3 is a different cause. That window had two providerStatuses pushes, and the rows rendered via state#0, which is useServerConfigs() (the row reads it for the pullRequests/pullRequestChecks capabilities). One render per row per push, not per poll. Reading just those two booleans through a selector would remove it, but that's outside this PR. The high totals in main #2 and PR #3 come from those pushes, on both sides.

Behaviour check: the rows and their states are unchanged: #19274, #19260, #19261 and #19270 show "Ready", and #19259 shows "Resolve". The checks requests still go out on schedule (Exit/Success responses arrive in every window).

Screenshot: /home/julius/.cache/audit/remeasure/20850-after.png