From 5f416fe0e7cee2509f049b042a2f10eeee8afc80 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 10:52:08 -0800 Subject: [PATCH 1/2] perf(chat): a live message renders one row, not the whole transcript Every row received V2Thread's aimAtMessageThread, which depended on messages, so each arrival handed every row a new callback and the whole transcript re-rendered: one 84-107ms frame per arrival on a phone at 4x CPU. The callback now reads messages through a ref and stays the same object, and V2MessageRow is memoized, so only the new row renders. The same arrival now measures 17-33ms between frames. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015AKgpMD6Qh13f7fj6mbGT3 --- .../V2TranscriptArrivalRender.test.tsx | 80 +++++++++++++++++++ frontend/src/v2/components/V2MessageRow.tsx | 5 +- frontend/src/v2/components/V2Thread.tsx | 9 ++- 3 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 frontend/src/v2/__tests__/V2TranscriptArrivalRender.test.tsx diff --git a/frontend/src/v2/__tests__/V2TranscriptArrivalRender.test.tsx b/frontend/src/v2/__tests__/V2TranscriptArrivalRender.test.tsx new file mode 100644 index 000000000..d7cc709e6 --- /dev/null +++ b/frontend/src/v2/__tests__/V2TranscriptArrivalRender.test.tsx @@ -0,0 +1,80 @@ +// @ts-nocheck +// A live message appends one row. The rows already on screen get the same +// props, so they must not render again: re-rendering the whole transcript cost +// one 84-107ms frame per arrival on a phone (4x CPU, 2026-11-01). +import React from 'react'; +import { render } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import V2Thread from '../components/V2Thread'; +import { AuthContext } from '../../context/AuthContext'; + +const mockAvatarRenders = []; +jest.mock('../../context/SocketContext', () => ({ useSocket: () => ({ socket: null, connected: false }) })); +jest.mock('../components/V2Avatar', () => { + const MockAvatar = ({ name }) => { mockAvatarRenders.push(name); return ; }; + MockAvatar.displayName = 'MockAvatar'; + return MockAvatar; +}); +jest.mock('../utils/avatars', () => ({ initialsFor: (name: string) => name.slice(0, 2) })); +const mockApi = { + get: jest.fn(() => Promise.resolve({})), + post: jest.fn(() => Promise.resolve({})), + put: jest.fn(() => Promise.resolve({})), + patch: jest.fn(() => Promise.resolve({})), + del: jest.fn(() => Promise.resolve({})), +}; +jest.mock('../hooks/useV2Api', () => ({ useV2Api: () => mockApi })); +beforeAll(() => { Element.prototype.scrollIntoView = jest.fn(); }); + +const authValue = { + currentUser: { _id: 'u1', username: 'viewer' }, + user: { _id: 'u1', username: 'viewer' }, + token: 't', + loading: false, + isAuthenticated: true, + error: null, + register: jest.fn(), login: jest.fn(), logout: jest.fn(), updateProfile: jest.fn(), +}; + +// Different authors an hour apart, so no row groups with the one before it. +const message = (id, author, hour) => ({ + id, pod_id: 'p1', user_id: `id-${author}`, content: `from ${author}`, message_type: 'text', + created_at: `2026-10-19T${String(hour).padStart(2, '0')}:00:00Z`, user: { username: author }, +}); +const history = [message('m1', 'ada', 9), message('m2', 'brook', 10), message('m3', 'cass', 11)]; + +const detail = { + pod: { _id: 'p1', name: 'Pod', type: 'chat' }, + members: [{ _id: 'u1', username: 'viewer', isBot: false }], + messages: history, + agents: [], + sendMessage: jest.fn(() => Promise.resolve({})), + loading: false, + error: null, + sendError: null, + hasMore: false, + loadingOlder: false, + loadOlder: jest.fn(() => Promise.resolve()), + refresh: jest.fn(), + initialLoadComplete: true, +}; + +const node = (d) => ( + + + + + +); + +test('a live arrival renders only the new row; the rows already shown stay as they are', () => { + const view = render(node(detail)); + const seen = (names) => mockAvatarRenders.filter((name) => names.includes(name)).length; + const before = seen(['ada', 'brook', 'cass']); + expect(before).toBeGreaterThanOrEqual(3); + + view.rerender(node({ ...detail, messages: [...history, { ...message('m4', 'dev', 12), liveArrivedAt: Date.now() }] })); + + expect(seen(['dev'])).toBeGreaterThanOrEqual(1); + expect(seen(['ada', 'brook', 'cass'])).toBe(before); +}); diff --git a/frontend/src/v2/components/V2MessageRow.tsx b/frontend/src/v2/components/V2MessageRow.tsx index ac8c06223..2c3573009 100644 --- a/frontend/src/v2/components/V2MessageRow.tsx +++ b/frontend/src/v2/components/V2MessageRow.tsx @@ -954,4 +954,7 @@ const V2MessageRow: React.FC = ({ message, sourceMessageId, d ); }; -export default V2MessageRow; +// Memoized: a live message appends one row, and every other row's props are +// unchanged, so only the new row renders. Callers keep the callbacks and maps +// they pass here stable (V2Thread's aimAtMessageThread reads messages by ref). +export default React.memo(V2MessageRow); diff --git a/frontend/src/v2/components/V2Thread.tsx b/frontend/src/v2/components/V2Thread.tsx index 76721dd1d..293bff802 100644 --- a/frontend/src/v2/components/V2Thread.tsx +++ b/frontend/src/v2/components/V2Thread.tsx @@ -340,14 +340,19 @@ const V2Thread: React.FC = ({ detail, firstRunVisible = false, in setReplyTarget(m); composerInputRef.current?.focus(); }, []); + // Read through a ref so the callback stays the same object while messages + // arrive: every message row receives it, and a new one each time would + // re-render the whole transcript for one new message. + const messagesRef = useRef(messages); + useLayoutEffect(() => { messagesRef.current = messages; }, [messages]); const aimAtMessageThread = useCallback((m: import('../hooks/useV2PodDetail').V2Message) => { // The action is available on every visible message, including replies. // Explicit roots may not themselves be inside a thread (#1128), so a // reply joins its existing root while a standalone message starts one. const rootId = String(m.thread_root_id ?? m.id); - const root = messages.find((candidate) => String(candidate.id) === rootId); + const root = messagesRef.current.find((candidate) => String(candidate.id) === rootId); aimAtThread(rootId, String(root?.content || m.content || '')); - }, [aimAtThread, messages]); + }, [aimAtThread]); // Memoized: it was called in the render body, so every keystroke in the // composer re-folded the whole message list. @sprint-review on #1150. From 120ecdb113d480c02728305015f52dd4c0d60ac4 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 10:56:00 -0800 Subject: [PATCH 2/2] fix(approvals): a parked call that expires on screen stops being decidable then The card decided "expired" from Date.now() during render, and only refreshed because the transcript happened to re-render. With rows memoized, an open pod could keep live Approve and Deny on a call that had already expired. The card now re-renders itself at the expiry moment. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015AKgpMD6Qh13f7fj6mbGT3 --- .../__tests__/V2ApprovalCardAction.test.tsx | 22 ++++++++++++++++++- frontend/src/v2/components/V2ApprovalCard.tsx | 14 ++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/frontend/src/v2/__tests__/V2ApprovalCardAction.test.tsx b/frontend/src/v2/__tests__/V2ApprovalCardAction.test.tsx index a8b7f296e..cc24f8ec5 100644 --- a/frontend/src/v2/__tests__/V2ApprovalCardAction.test.tsx +++ b/frontend/src/v2/__tests__/V2ApprovalCardAction.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import { MemoryRouter, useLocation } from 'react-router-dom'; import V2ApprovalCard from '../components/V2ApprovalCard'; import en from '../../i18n/locales/en.json'; @@ -203,6 +203,26 @@ describe('tool-call approval details stay owner-scoped', () => { expect(screen.getByText(en.approvalCard.decline)).toBeDisabled(); }); + test('a parked call that expires while the card is on screen stops being decidable at that moment', () => { + jest.useFakeTimers(); + // The owner's card reads the pending index while the call is live. + mockGet.mockReturnValue(new Promise(() => {})); + try { + renderCard({ + kind: 'approval-card', approvalId: 'tool-expiring', status: 'flagged', + ownerUserId: 'viewer-1', actionType: 'tool_call', toolCall: { tool: 'github.create_issue' }, + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }); + expect(screen.queryByText(en.approvalCard.toolCall.expiredPending)).not.toBeInTheDocument(); + act(() => { jest.advanceTimersByTime(60_001); }); + expect(screen.getByText(en.approvalCard.toolCall.expiredPending)).toBeInTheDocument(); + expect(screen.getByText(en.approvalCard.approve)).toBeDisabled(); + expect(screen.getByText(en.approvalCard.decline)).toBeDisabled(); + } finally { + jest.useRealTimers(); + } + }); + test('a resolved tool call renders its broker ending and code instead of generic success', () => { renderCard({ kind: 'approval-card', approvalId: 'tool-failed', status: 'resolved', decision: 'approved', diff --git a/frontend/src/v2/components/V2ApprovalCard.tsx b/frontend/src/v2/components/V2ApprovalCard.tsx index 9e0f4f9f8..e82dfb89e 100644 --- a/frontend/src/v2/components/V2ApprovalCard.tsx +++ b/frontend/src/v2/components/V2ApprovalCard.tsx @@ -103,8 +103,18 @@ const V2ApprovalCard: React.FC = ({ message, authorLabel, t const status = payload.status || 'flagged'; const isOwner = !!currentUser?._id && String(currentUser._id) === String(payload.ownerUserId || ''); const isToolCall = payload.actionType === 'tool_call'; - const aged = status === 'flagged' && !!payload.expiresAt - && new Date(payload.expiresAt).getTime() < Date.now(); + // The card re-renders itself at the expiry moment. Its row is memoized, so + // nothing else is guaranteed to render it again while the pod stays open. + const expiresMs = status === 'flagged' && payload.expiresAt ? new Date(payload.expiresAt).getTime() : NaN; + const [expiryTick, setExpiryTick] = useState(0); + useEffect(() => { + const wait = expiresMs - Date.now(); + if (!Number.isFinite(wait) || wait < 0) return undefined; + // setTimeout caps near 24.8 days; a longer wait re-arms on the tick. + const timer = window.setTimeout(() => setExpiryTick((n) => n + 1), Math.min(wait + 1, 2 ** 31 - 1)); + return () => window.clearTimeout(timer); + }, [expiresMs, expiryTick]); + const aged = Number.isFinite(expiresMs) && expiresMs < Date.now(); const needsToolCall = isOwner && status === 'flagged' && isToolCall && !aged; const [pendingToolCall, setPendingToolCall] = useState(null); const [toolCallLoading, setToolCallLoading] = useState(false);