[rgodha24]

clank context (clickme)

Follow-up to #37001 and #37002.

Problem

Streaming in a thread with many linked pull requests repeats work over every link on each shell tick. While a run streams, the server resends the thread's whole shell several times a second, and every linked pull request arrives as a freshly decoded object. Each tick then:

• deep-compares every link through the Schema equivalence (sameThreadShell, sameApartFromStreaming),
• re-resolves the current pull request chains from scratch in each component that reads them (SidebarThreadRow, ThreadStatusIndicators, threadReference, pullRequestDetail.logic),
• rebuilds savedPullRequestSummariesAtom from every link in the environment. Each link's URL is parsed in threadPullRequestLinkToSummary, and its key is normalized again because savedPullRequestKey wraps it in a fresh object, so the WeakMap key cache misses. This atom rebuilds on any thread's tick, not just the streaming one.

The thread this came from has 322 linked PRs; the environment has 1,340 links across 312 threads.

Change

• shellReducer: a thread.updated delta, or a snapshot row, whose links equal the prior row's keeps the prior links array. Later equivalence checks then stop at identity, and per-link caches stay warm.
• resolveThreadCurrentPullRequest: cached per links array in a WeakMap (arrays are immutable).
• savedPullRequestSummariesAtom: keys by the link itself, so the existing key cache hits, and caches each link's summary per link object.

Behavior is unchanged: a changed link yields a new array, so every cache recomputes.

Scope

A real performance problem for long-running threads that link many PRs (stacks, sweeps). client-runtime and source-control-core are shared with mobile, so the reducer and current-PR changes apply there too. savedPullRequestSummariesAtom is web-only.

Verification

Real data, not fixtures: a copy of the actual 322-PR thread (22,900 items, 486-thread family) and its environment, made with migrate-dev-db. The server and browser ran inside a sandbox (bubblewrap with no network, read-only filesystem, provider credentials hidden), so nothing could reach the real worktree or a provider. The app was a production build of current main with sourcemaps. In Playwright I switched the thread's composer to the Replay (dev) Showcase model, streamed one turn, and captured a CDP trace with the V8 CPU profiler. Base and branch builds ran alternately, 3 runs each, with CPU profiles symbolicated through the build's sourcemaps:

one streamed turn	base	this PR
main-thread busy	1,262 / 1,193 / 1,236 ms	1,103 / 865 / 1,097 ms
Schema equivalence (toEquivalence)	84 / 78 / 79 ms	43 / 45 / 43 ms
Schema decode (SchemaParser)	93 / 89 / 91 ms	60 / 61 / 60 ms
resolveThreadCurrentPullRequest	60 / 59 / 62 ms	24 / 25 / 24 ms
savedPullRequestSummariesAtom	36 / 38 / 38 ms	3 / 3 / 3 ms

A reducer-only build reached about 1,150 ms busy and left the summaries rebuild at about 37 ms and current-PR resolution at about 35 ms. So all three changes carry weight.

• The longest task is the same in every run (about 250 ms). It is the profiler starting and has no samples.
• shellReducer.test.ts gets one case: a changed row with equal links keeps the prior array on both the single and batched paths, and a changed link replaces it. It fails on main and passes here.
• vp run check:changed passes (19 packages typechecked).
• Not measured: several agents streaming at once, which multiplies the environment-wide summaries rebuild; the mobile client.

[macroscopeapp]

Approvability

Verdict: Approved at 3a58a7e

Macroscope's review found this PR approvable — This change reuses unchanged PR links and caches derived PR summaries during streaming. Its effects are confined to client-side computation and presentation, with no product-default or persistent-data changes.

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

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: c910b1b
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0f0d1980-0442-45cd-b413-928a9cd3e0be

Commits

Reviewing files that changed from the base of the PR and between b90e6ad and 77cb885.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Thread updates now retain equivalent pull-request link arrays. Pull-request resolution and saved-link summary conversion cache derived results.

Changes: Pull-request link handling

Layer / File(s)	Summary
Retain equivalent link arrays packages/client-runtime/src/state/shellReducer.ts, packages/client-runtime/src/state/shellReducer.test.ts	Snapshot and streaming updates reuse existing pull-request arrays when their contents are equivalent. Tests cover single and batched updates, including changed links.
Cache current pull-request resolution packages/source-control-core/src/shared/threadPullRequests.ts	Resolution results, including null, are cached by links-array identity.
Cache saved-link summaries apps/web/src/state/pullRequests.ts	Converted summaries are cached by link identity and project ID. The aggregation uses the cached summaries and continues to skip null results.

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

Change: Refactor

Suggested reviewers: saphid, juliusmarminge

Merge Risk: Minimal · previous review 3a58a

The change reuses unchanged pull-request link arrays and caches derived results to cut repeated work while streaming. No concrete regression was found, and the reducer tests cover both equal and changed links. The failing CI test is an unrelated port conflict.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "perf(client): reuse unchanged pr links while streaming" matches the main change. The PR keeps the prior links array when streamed thread rows have equal links, and it adds caches for curren…
Description check		The description follows the repository template. It has Problem, Change, Scope, and Verification sections, and it uses a conventional-style summary. The Problem section explains the repeated work duri…
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.

Full details: Title check

Explanation

The title "perf(client): reuse unchanged pr links while streaming" matches the main change. The PR keeps the prior links array when streamed thread rows have equal links, and it adds caches for current pull request resolution and saved summaries. The title is short, uses a conventional commit prefix, and names the primary goal. It does not mention the caching of current-PR resolution or summaries, but those are secondary aspects.

Full details: Description check

Explanation

The description follows the repository template. It has Problem, Change, Scope, and Verification sections, and it uses a conventional-style summary. The Problem section explains the repeated work during streaming, with the affected components and atoms. The Change section lists the three modifications. The Scope section names the shared mobile impact and the web-only atom, and it links the related PRs #37001 and #37002 as context. It does not link a bug issue or a maintainer request, which the template asks for. The Verification section gives concrete measurements, a regression test, and a check run. It also states what was not measured. The description includes a 'clank context' details wrapper that is not in the template. The description has no agent/model line at the end, although the template asks for one if an agent was used. These gaps are minor.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: bdf6c6e
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: dd39aa15-f8ed-4ade-9be9-76afd0599046

Commits

Reviewing files that changed from the base of the PR and between bdf6c6e and 3a58a7e.

Files selected for processing: 4

• apps/web/src/state/pullRequests.ts
• packages/client-runtime/src/state/shellReducer.test.ts
• packages/client-runtime/src/state/shellReducer.ts
• packages/source-control-core/src/shared/threadPullRequests.ts

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

[juliusmarminge]

The automated pipeline stopped on this pull request and will not merge it:

in the real app the problem is still there, and the rework changed nothing:

No code change: I found no defect. The report says the problem still happens but gives no numbers and names no wrong behavior. I checked the mechanism directly. A throwaway benchmark (not committed) used the proof's fixture shape (312 threads, 1,340 links, 322 on the streaming thread) and ran 200 thread.updated ticks, each carrying freshly copied links. On main the streaming row's links array was replaced on all 200 ticks. On this branch it was kept on all 200, and resolving the current PR for every row fell from about 430 ms to 14 ms; reducer cost was the same on both sides, about 95 ms. I traced the real app's paths. Stream deltas carry each thread's full row with its links (ShellStream) and go through applyShellStreamEvents. resolveThreadCurrentPullRequest is one shared instance for every web and mobile caller. threadShellsAtom and quietThreadShellAtomFamily reuse the source rows' links arrays. So the caches are reached in the real app. Main has not touched the involved files since the last merge, so no merge was needed. The PR never claimed to remove the streaming slowdown: its own profile showed main-thread busy time per streamed turn going from about 1,230 to 1,020 ms (decode, Schema equivalence and rendering remain). A total-busy-time comparison on a synthetic fixture with few shell ticks per turn can see that gain as noise. If the bar is removing the slowdown, that needs a separate, larger change, not a fix to this branch. Nothing was pushed, so I did not re-watch the PR; its checks had already passed on this head.

It stays open for a person to finish or close.