Repository navigation
perf(channels): keep openLink stable across tab changes - #1525
Conversation
- the workspace openLink callback depended on the whole tab state object, which changes identity on every tab update such as opening a thread - each new openLink identity defeated MessageRow memoization, so every mounted timeline row re-rendered on thread open - depend on the stable setTabs, select and leaveVisit actions instead Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Kalvin Chau <[email protected]>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/performance/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/accessibility/focus) reviews.
No actionable findings. Source-level assessment: 9/10 for minimalness, ownership and correctness.
The narrowed dependencies cover every tab-state action used by openLink. Those actions read current state through functional updates and change identity with the channel/session owner. Destination and route refs, connection/abort checks, and focus capture remain current at invocation. The callback reaches memoized message rows, supporting the intended reduction in unnecessary renders without changing navigation behavior.
Reviewed head: cdc6275
Base: 3d1938a
Scope: the complete one-file diff, tab-store actions, relevant consumers and lifecycle/focus paths, public PR description and commit metadata. No attached images or disclosure findings. No browser journeys were added or removed.
Validation limits: source-only; no tests, builds, app launches or benchmark reproduction. The 02:18 UTC hosted-check snapshot showed CI required and all executed checks successful; Windows native validation and flake-check jobs were skipped. The PR reports five local full-suite timeout failures and successful isolated reruns; those results and the reported performance improvements were not independently reproduced. Live keyboard/screen-reader behavior, native acceptance, visual checks and human acceptance remain unverified. This COMMENT is not an approval or merge authorization.
…r-emoji-size * origin/main: (26 commits) perf(profiles): notify once per batch of owner attestations (#1650) feat(pi): load Pi durable extensions and providers from Pi packages (#1610) Teach agents memory links and source metadata; frame memory as information (#1562) Make avatar choices visible in agent creation (#1631) fix(pi): take plugin wakes instead of ignoring them (#1612) perf(agents): parse activity records once per snapshot (#1449) perf(terminal): parse hidden terminal output in slices (#1459) perf(composer): reuse the document projection for editor reads (#1463) perf(read-state): load peer journals without a strict writeback (#1448) perf(channels): keep openLink stable across tab changes (#1525) fix: name the Cargo package Buzz so the dev build shows "Buzz" (#789) test(browser): re-type a dropped page name in the search palette (#1635) test(browser): fix two main CI timing failures (#1567) feat(agents): deliver Attention watches and timers to conversations (#1578) fix(inbox): stop listing tag-only composer state as drafts (#1531) Choose Builderlab agent models at creation and in Settings (#1533) feat(communities): sync known communities through Builderlab accounts (#644) Tag the last-tagged agent when a Solo chat composer is shown empty (#1615) perf(usage): read Claude usage once, then only what changed (#1586) feat(chats): ask to share access when sending a chat link into a DM (#1614) ...
Opening a thread re-rendered every mounted message row in the channel timeline, even though none of them changed.
The workspace
openLinkcallback inChannelWorkspacelisted the wholetabStateobject as a dependency. That object changes identity on every tab update, and opening a thread updates the tabs, so each click created a newopenLink. The new identity reached everyMessageRowand defeated itsmemo. The callback only callssetTabs,selectandleaveVisit, which are stable per channel, so it now depends on those instead.Measurements
Browser fixture: a 60-message channel, one "View thread" click on a visible root, Chromium, 4x CPU throttle. Each case was measured in two alternating rounds. Values are median [min–max].
Production build, 12 samples per case:
Development React, where render cost is magnified, 10 samples per case:
React commit instrumentation on the same click, development React: component renders went from 5308 to 3903.
MessageRowrenders went from 42 to 8, and the remaining 8 are thread-panel rows.Testing
tsc --noEmit,biome checkvitest run: the full suite had 5 failures under load, all timeouts in unrelated files (inbox, notification settings, codex, relay startup, message edit). Those files pass when run on their own.src/bundled/channelsandsrc/features/messagespass after the rebase.buzz-links,channel-tabs,external-links,navigation-thread-history,tab-close-others,thread-follow,thread-opening,thread-tab-moves,thread-unread,thread-window(51 passed)🤖 Generated with Claude Code