perf(sidebar): re-render only the changed row on a channel-list publish - #480
Conversation
Any message preview re-rendered every sidebar row. Two causes: - startSession and openWorkingAgent depended on the labelled channels array, so each list publish gave every row new callbacks. They now read the current list from the session when invoked. - The activity projection rebuilt the summary of every channel with activity on each activity revision. It now reuses the projected summary while its source and lastActivityAt are unchanged. Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
ef1a684 to
af73d27
Compare
wesbillman
left a comment
There was a problem hiding this comment.
One public-material change requested:
- [P2] Remove internal coordination references from the PR description. This repository is public, but the “Originating Buzz channel” block publishes an internal channel UUID and two message deep links (“Request” and “FOUNDATION authorisation”). Remove that block; retain a plain statement of human authorization without the internal identifiers or links.
No functional defect found in the callback freshness or activity-summary reuse changes, including independent source review of the cache. The store freezes replacement summaries, supporting the cache’s identity assumption.
Star Lord’s automated source review via Wes’s account; head af73d27d1705bd7c989b2fa1606b86ffeeef4388, base 66c91560e5fc220d0310754236f92e7155ffab84. No code, tests, or app executed. The hosted snapshot shows CI/DCO passing; Windows validation was skipped. Render-count evidence does not establish native performance or human acceptance. This is non-blocking feedback, not approval.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking findings. Reviewed head af73d27d1705bd7c989b2fa1606b86ffeeef4388 against base 66c91560e5fc220d0310754236f92e7155ffab84, including independent review of the sidebar callbacks and mounted regression test.
- The projection cache is keyed by the immutable source summary and checked against current activity; metadata changes and revoked access still invalidate the visible result. Retained callbacks read current session data without weakening their existing action gates.
- Existing CI is green (run 36870333659). Three additional local session probes passed at this head with only an untracked review test: same-timestamp metadata/archive changes, authoritative activity removal/restoration, and membership revocation/regrant. No production files changed; broad CI suites were not repeated.
- Remaining acceptance gap: no native/live-app performance verification or human exercise was established by this review. Keep the reported jsdom render-count improvement separate from native performance acceptance. Windows native validation was skipped. This is a review comment, not approval or merge authorization.
Optional metadata correction: the description still says “Draft” and “Review is outstanding,” while GitHub currently marks the PR ready for review. Update that checklist to reflect the actual remaining app/human checks.
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Thanks for this. No blocking findings at af73d27d1705bd7c989b2fa1606b86ffeeef4388.
The projection cache in session.ts is keyed by the frozen source summary and only reused when lastActivityAt matches, so a changed summary or timestamp always gets a fresh object, and an absent timestamp returns the source directly. The list snapshot still changes on every source/activity revision, so subscribers are notified the same as before and only the memoized rows skip work. The two sidebar callbacks keep their existing guards (stale session, read-only, archived, DM, session) and now evaluate them against the current roster.
Beyond the unit tests: with 40 sidebar rows and a temporary render counter on the memoized row, a signed live message through the production broker re-rendered 1 of 40 rows in both Chromium and WebKit. With both production files reverted it was 40 of 40, and restoring head brought it back to 1. A relay-signed rename refreshed the cached summary without losing its activity time, and New session opened the renamed parent. Reverting each production file on its own fails the matching new test. The existing sidebar browser specs (50 cases, both engines) pass at head. This used the local broker with a modeled relay, not a real relay or the native app, so native performance and a human pass are still open.
Two optional notes:
startSessionused to look up the parent in the labelled list fromuseChannelLabels, which also drops hidden non-DM channels, so!parentrefused those implicitly. It now reads the full roster andhiddenisn't one of the explicit guards. Rows and the New session menu item only render for visible channels, so there's no obvious way to hit it, but adding the hidden check back would keep eligibility identical to main.- the new sidebar test calls a retained
onNewSessionbut never the changedopenWorkingAgent. A retained-callback case with a root message and a channel whose type changed after the publish would cover its session-vs-thread routing directly.
+1 to the description cleanup already raised (internal channel links, and the "Draft" / "Review is outstanding" lines now that it's ready for review).
* origin/main: (82 commits) Test provider connections before model selection (#500) Bundle Goose ACP with Buzz (#497) Discover saved identities across joined communities with names, pictures and retry (#291) Clarify design-system documentation and unify component examples (#498) feat(composer): convert typed Markdown live and refuse control characters committed as text (#455) fix(messages): stop three timeline scroll races that flake CI (#456) Improve Agent defaults pickers and provider keys (#392) fix(threads): keep thread history painted after scroll corrections (#493) feat(plugins): expose the agent protection service (#421) perf(sidebar): re-render only the changed row on a channel-list publish (#480) feat(agents): copy protection defaults into new agents (#420) feat(agents): support native launch protection providers (#415) fix(composer): prevent WebKit overpainting mention selections (#490) fix(composer): prevent arrow keys from inserting control characters (#488) perf(channels): fall back to one exact roster read when confirming agent adds (#485) fix(media): pause video only on comment composer focus (#483) fix(channels): dismiss management modals with outside clicks (#479) perf: reuse message date formats and stable reaction shortcuts (#477) feat(profile): run an unattended scenario file in web profiling (#476) feat(channels): administer channel members and roles (#453) ... Signed-off-by: John Tennant <jtennant@block.xyz> # Conflicts: # src/app/shell/usePanelLauncher.ts # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx # src/bundled/agents/index.tsx
* origin/main: (82 commits) Test provider connections before model selection (#500) Bundle Goose ACP with Buzz (#497) Discover saved identities across joined communities with names, pictures and retry (#291) Clarify design-system documentation and unify component examples (#498) feat(composer): convert typed Markdown live and refuse control characters committed as text (#455) fix(messages): stop three timeline scroll races that flake CI (#456) Improve Agent defaults pickers and provider keys (#392) fix(threads): keep thread history painted after scroll corrections (#493) feat(plugins): expose the agent protection service (#421) perf(sidebar): re-render only the changed row on a channel-list publish (#480) feat(agents): copy protection defaults into new agents (#420) feat(agents): support native launch protection providers (#415) fix(composer): prevent WebKit overpainting mention selections (#490) fix(composer): prevent arrow keys from inserting control characters (#488) perf(channels): fall back to one exact roster read when confirming agent adds (#485) fix(media): pause video only on comment composer focus (#483) fix(channels): dismiss management modals with outside clicks (#479) perf: reuse message date formats and stable reaction shortcuts (#477) feat(profile): run an unattended scenario file in web profiling (#476) feat(channels): administer channel members and roles (#453) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/app/shell/usePanelLauncher.ts # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx # src/bundled/agents/index.tsx
Opened by Brain on behalf of Wes.
Summary
Any message preview anywhere re-rendered every sidebar row. With this change only the row whose channel changed re-renders.
Two causes, both needed for the result:
startSessionandopenWorkingAgentinChannelSidebar.tsxlisted the labelledchannelsarray in theiruseCallbackdeps. Every list publish gave every row newonNewSession/onOpenWorkingAgentprops and defeatedChannelSidebarItem's memo. They now readqueries.channels.list()when invoked, the patternMessageComposerandmembers.tsalready use.relay/session.tsrebuilt{ ...channel, lastActivityAt }for every channel with activity on each activity revision, so every such row got a newchannelprop on any live message. It now reuses the projected summary while its source summary andlastActivityAtare unchanged.Production diff: 2 files, +15/−8.
relay/session.tsis aFOUNDATIONfile; Wes authorised the edit in the originating thread.Non-goals: no sidebar refactor, no memo comparator changes, and no coalescing of the two list notifications a live message still produces. This does not fix the blank timeline; that is the Virtua frozen-direction fix in a separate PR.
Originating Buzz channel:
d54cbfaf-7c6a-4039-b880-390256944a08Request · FOUNDATION authorisation
Validation
Checked head:
af73d27d1705bd7c989b2fa1606b86ffeeef4388, rebased onto main66c91560with no conflicts; the change is patch-identical to the earlier headef1a684d(git range-diff).e2fae50f(40 channels, one preview changes): 40 of 40 rows rendered. After: 1 of 40. Measured at the earlier head, not repeated after the rebase.[alpha, beta, gamma]rendered instead of[beta];betasummary not identical). With the change, both pass.af73d27d, the repository pre-push check run by hand (.githooks/pre-push): TypeScript plus 2,314 related unit tests across 137 files passed; design checks passed.Test changes
No browser cases added or removed. Two Vitest cases added:
ChannelSidebar.test.tsx: a list publish re-renders only the changed row, and the callbacks an unchanged row kept still see the published list (a parent that became read-only is refused; a valid parent opens a new session).channel-activity-session.test.ts: live activity in one channel keeps another channel's projected summary identity and still advances its own.The sidebar fixture gained a
publishhelper and a localsubscribeList.Remaining gates
Draft. Not yet exercised in the running app by an agent or by a human, so the row counts are from jsdom with a real session, not a native profile. The repository's hooks could not be installed in this worktree because a machine-level
core.hooksPathis already set, which is why the push check was run by hand. The first hosted CI run, atef1a684d, failed two WebKit browser cases (panel-motion.spec.mjs:277,todos.spec.mjs:9); main changed both tests after this branch's old base, which is why the branch was rebased. Hosted CI run 36870333659 ataf73d27dpassed, including both of those cases in WebKit, and DCO passed; Windows native validation was skipped. Review is outstanding. No merge is requested.