diff --git a/frontend/src/app/components/editor/ElementSelectionContext.tsx b/frontend/src/app/components/editor/ElementSelectionContext.tsx index 1109a82c..be025a4d 100644 --- a/frontend/src/app/components/editor/ElementSelectionContext.tsx +++ b/frontend/src/app/components/editor/ElementSelectionContext.tsx @@ -136,28 +136,29 @@ export const ElementSelectionProvider: React.FC<{ children: React.ReactNode }> = }); }, []); + // One value identity per state change: the inline literal was a new object on every provider render, and every + // browser card reads this context, so all of them re-rendered on each render of the app shell (a send, for one). + const value = useMemo(() => ({ + selectMode, + toggleSelectMode, + setSelectMode, + excludeSelectId, + setExcludeSelectId, + activeOwnerId, + setActiveOwnerId, + selectedElements, + addSelectedElement, + updateSelectedElement, + removeSelectedElement, + clearSelectedElements, + elementsByOwner, + addElementForOwner, + removeOwnerElement, + clearOwnerElements, + iframeRef, + }), [selectMode, toggleSelectMode, excludeSelectId, activeOwnerId, selectedElements, addSelectedElement, updateSelectedElement, removeSelectedElement, clearSelectedElements, elementsByOwner, addElementForOwner, removeOwnerElement, clearOwnerElements]); return ( - + {children} ); diff --git a/frontend/src/app/pages/AgentChat/AgentChat.tsx b/frontend/src/app/pages/AgentChat/AgentChat.tsx index 3352d743..262a173f 100644 --- a/frontend/src/app/pages/AgentChat/AgentChat.tsx +++ b/frontend/src/app/pages/AgentChat/AgentChat.tsx @@ -397,6 +397,9 @@ const AgentChat: React.FC = ({ sessionId: sessionIdProp, onClose const windowEndRef = useRef(0); const windowScrollRafRef = useRef(null); const [viewportHeight, setViewportHeight] = useState(0); + // Bubbles read the height only for the oversized-message threshold, so hand them an 80 px bucket: the scroller's + // clientHeight moves a few px as the composer and working slot change, and the raw value re-rendered every bubble per message. + const bubbleViewportHeight = Math.round(viewportHeight / 80) * 80; const [viewportWidth, setViewportWidth] = useState(0); const [scrollRoot, setScrollRoot] = useState(null); const [windowStart, setWindowStart] = useState(0); @@ -2077,7 +2080,7 @@ const AgentChat: React.FC = ({ sessionId: sessionIdProp, onClose editing={isEditing} onSaveEdit={handleSaveEdit} onCancelEdit={handleCancelEdit} - viewportHeight={viewportHeight} + viewportHeight={bubbleViewportHeight} viewportWidth={viewportWidth} scrollRoot={scrollRoot} /> diff --git a/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx b/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx index cbdb54cb..93992108 100644 --- a/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx +++ b/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx @@ -1365,15 +1365,18 @@ const ChatMessageBubble: React.FC = ({ message, editing = false, onSaveEd Waiting for the current step to finish. Press Stop to send it now. )} - setPickerOpen(false)} - title="Upgrade your plan" - subtitle="Pick a plan to keep going. Cancel anytime from Stripe." - source="upgrade_cta" - defaultPlan="pro_plus" - onSubscribed={() => setPickerOpen(false)} - /> + {/* Mounted only while open: every bubble carried a closed modal, so a 40-message transcript ran 40 modal renders per streamed message. */} + {pickerOpen && ( + setPickerOpen(false)} + title="Upgrade your plan" + subtitle="Pick a plan to keep going. Cancel anytime from Stripe." + source="upgrade_cta" + defaultPlan="pro_plus" + onSubscribed={() => setPickerOpen(false)} + /> + )} ); }; diff --git a/frontend/src/app/pages/Dashboard/canvas/DashboardCanvas.tsx b/frontend/src/app/pages/Dashboard/canvas/DashboardCanvas.tsx index 7e1e09f1..c74347ab 100644 --- a/frontend/src/app/pages/Dashboard/canvas/DashboardCanvas.tsx +++ b/frontend/src/app/pages/Dashboard/canvas/DashboardCanvas.tsx @@ -1,10 +1,11 @@ -import React, { useEffect, type RefObject } from 'react'; +import React, { useEffect, type RefObject, useMemo } from 'react'; import Box from '@mui/material/Box'; import { useAppDispatch, useAppSelector } from '@/shared/hooks'; import { addViewCard, addBrowserTab, clearTiledCard, toggleMinimizeCard, selectFullscreenCardId, selectViewportCoveringCardId } from '@/shared/state/dashboardLayoutSlice'; import { store } from '@/shared/state/store'; import { buildDockEntries } from '../desktop/dockEntries'; import DashboardHeader from './DashboardHeader'; +import type { HeaderSession } from './DashboardHeader'; import TetherLayerHost from './TetherLayerHost'; import { useLiveMultiDrag } from '../hooks/interaction/useLiveMultiDrag'; import DashboardCardLayer from './DashboardCardLayer'; @@ -346,6 +347,14 @@ const DashboardCanvas: React.FC = ({ canvas.actions.syncTransform(); }); + // The header lists id, name, status and model; keyed on those values so a streamed message (a new sessions map) re-lists nothing. + const headerSig = Object.values(sessions).map((s) => `${s.id}|${s.name}|${s.status}|${s.model}`).join('\n'); + const headerSessions = useMemo>(() => { + const out: Record = {}; + for (const s of Object.values(sessions)) out[s.id] = { id: s.id, name: s.name, status: s.status, model: s.model }; + return out; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [headerSig]); return ( <> @@ -377,7 +386,7 @@ const DashboardCanvas: React.FC = ({ ; + interface DashboardHeaderProps { dashboardName: string | undefined; - sessions: Record; + // Only the fields the list reads: the whole sessions map re-rendered every row on every streamed message. + sessions: Record; cards: Record; viewCards: Record; browserCards: Record; @@ -119,6 +123,7 @@ const DashboardHeader: React.FC = ({ }, [canvasActions, onHighlightCard], ); + const focusStable = useStableCallback(handleFocus); const toggle = useCallback(() => { if (hasItems) setExpanded((v) => !v); @@ -272,29 +277,7 @@ const DashboardHeader: React.FC = ({ {agentItems.length > 0 && ( } label="Agents" count={agentItems.length} c={c}> {agentItems.map((item) => ( - handleFocus(item.id, item.card)} c={c}> - - - {item.name} - - - {friendlyStatusLabel(item.status)} - - + ))} )} @@ -379,6 +362,21 @@ const CategoryGroup: React.FC<{ ); +// One row per agent on primitives, memoised: the list rebuilt every row's closure on every header render, and the header +// renders whenever a session's name or status moves, so a 60-card board paid 60 row renders per event. +const AgentRow = React.memo(({ id, name, status, x, y, width, height, onFocus, c }: { + id: string; name: string; status: string; x: number; y: number; width: number; height: number; + onFocus: (cardId: string, card: { x: number; y: number; width: number; height: number }) => void; + c: ReturnType; +}) => ( + onFocus(id, { x, y, width, height })} c={c}> + + {name} + {friendlyStatusLabel(status)} + +)); +AgentRow.displayName = 'AgentRow'; + const ItemRow: React.FC<{ onClick: () => void; c: ReturnType; diff --git a/frontend/src/app/pages/Dashboard/cardCallbacksAreStable.test.ts b/frontend/src/app/pages/Dashboard/cardCallbacksAreStable.test.ts new file mode 100644 index 00000000..9773b560 --- /dev/null +++ b/frontend/src/app/pages/Dashboard/cardCallbacksAreStable.test.ts @@ -0,0 +1,23 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync, existsSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +// ENG-487 / ENG-467: on a send the selection object is rebuilt and the welcome flag flips, and four callbacks handed to every +// memoized AgentCard changed identity with them, so the whole board re-rendered (4,800 fibers, 380 ms at 4x). Each of the four +// is wrapped in useStableCallback now; the profiler (commit_profile.mjs) is the liveness proof, this pins the wiring. +const here = fileURLToPath(new URL('.', import.meta.url)); +const src = here.replace(/([\\/])\.test-build([\\/])/, '$1src$2'); +const files = { + 'hooks/interaction/useDashboardInteractions.ts': ['handleCardSelect'], + 'hooks/interaction/useCardDrag.ts': ['handleCardDragStart', 'handleCardDragEnd'], + 'hooks/lifecycle/useAgentSpawn.ts': ['handleBranchFromCard'], +}; +for (const [rel, names] of Object.entries(files)) { + test(`${rel}: ${names.join(', ')} keep one identity across renders`, () => { + const path = src + rel; + assert.ok(existsSync(path), `could not locate ${rel} from ${here}`); + const text = readFileSync(path, 'utf8'); + for (const n of names) assert.match(text, new RegExp(`const ${n} = useStableCallback\\(${n}Impl\\);`), n); + }); +} diff --git a/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx b/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx index 1541fa7e..b977f696 100644 --- a/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx +++ b/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx @@ -307,31 +307,18 @@ const AgentCard: React.FC = ({ const expanded = expandedInStore || isTiled; const isDashboardActive = useDashboardActive(); const hasApiKey = !!useAppSelector((s) => s.settings.data.anthropic_api_key); - const expandedSessionIds = useAppSelector((s) => s.agents.expandedSessionIds); const workflowSuggestion = useMemo(() => findWorkflowSuggestion(session), [session]); // Suppress the convert-suggestion glow when this chat is already entangled with a workflow. Two cases: (a) The session is one of a workflow's runner sessions, OR (b) The session is the source the workflow was originally derived from. Either way a fresh convert would just clone the workflow, which is confusing identity collapse. - const workflowRunsMap = useAppSelector((s) => s.workflows.runs); - const workflowItems = useAppSelector((s) => s.workflows.items); const linkedWorkflowSidecarId = useAppSelector((s) => { const entry = Object.values(s.workflows.openCards).find((card) => card.sidecarSessionId === session.id); return entry?.workflowId ?? null; }); - const sourceWorkflow = useMemo(() => { - for (const wf of Object.values(workflowItems || {})) { - if (wf.source_session_id === session.id) return wf; - } - return null; - }, [workflowItems, session.id]); - const isWorkflowRunnerSession = useMemo(() => { - // A Test Agent (spawned to validate a workflow draft) isn't a chat to convert; it carries workflow_test_state. - if (session.workflow_test_state) return true; - for (const arr of Object.values(workflowRunsMap || {})) { - for (const r of arr || []) { - if (r.session_id === session.id) return true; - } - } - return Boolean(sourceWorkflow); - }, [workflowRunsMap, sourceWorkflow, session.id, session.workflow_test_state]); + // Per-card answers, not the whole workflow maps: subscribing every card to `workflows.items` and `workflows.runs` + // re-rendered the entire board on any run update. The object is a stable reference until that workflow changes. + const sourceWorkflow = useAppSelector((s) => Object.values(s.workflows.items || {}).find((wf) => wf.source_session_id === session.id) ?? null); + const hasWorkflowRun = useAppSelector((s) => Object.values(s.workflows.runs || {}).some((arr) => (arr || []).some((r) => r.session_id === session.id))); + // A Test Agent (spawned to validate a workflow draft) isn't a chat to convert; it carries workflow_test_state. + const isWorkflowRunnerSession = Boolean(session.workflow_test_state) || hasWorkflowRun || Boolean(sourceWorkflow); const hasUserPrompt = useMemo( () => session.messages.length > 0 ? session.messages.some((m) => m.role === 'user' && !m.hidden) @@ -375,13 +362,14 @@ const AgentCard: React.FC = ({ return; } if (scheduleWorkflowCount <= baselineScheduleCountRef.current) return; - for (const wf of Object.values(workflowItems || {})) { + // The count is the trigger; the list is read when it fires, so the map is not a subscription. + for (const wf of Object.values(store.getState().workflows.items || {})) { if (wf.source_session_id !== session.id) continue; if (autoOpenedWorkflowIdsRef.current.has(wf.id)) continue; autoOpenedWorkflowIdsRef.current.add(wf.id); dispatch(openWorkflowsApp({ workflowId: wf.id })); } - }, [scheduleWorkflowCount, workflowItems, session.id, dispatch]); + }, [scheduleWorkflowCount, session.id, dispatch]); const cardBoxRef = useRef(null); // Ref so ResizeObserver sees latest value without re-attaching when active flips. @@ -792,7 +780,8 @@ const AgentCard: React.FC = ({ onContextMenu={(e: React.MouseEvent) => { if (isNativeMenuTarget(e)) return; if ((e.target as HTMLElement).closest?.('[data-chat-transcript]')) return; openCardContextMenu(e, { rename: { value: displayChatTitle(session), onCommit: (name) => dispatch(renameSession({ sessionId: session.id, name })) }, items: agentCardMenuRows({ - session, dispatch, expanded, tileZone, expandedSessionIds, + // Read at click time: subscribing every card to the expanded list re-rendered the whole board on each expand (a send expands the new chat). + session, dispatch, expanded, tileZone, expandedSessionIds: store.getState().agents.expandedSessionIds, card: { x: cardX, y: cardY, width: cardWidth, height: cardHeight }, onTile, onClose: () => handleRemove(), }), diff --git a/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx b/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx index 78493b5a..1e5c00d0 100644 --- a/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx +++ b/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx @@ -1073,8 +1073,8 @@ const BrowserCard: React.FC = ({ const accentColor = c.accent.primary; - const glowingBrowserCards = useAppSelector((s) => s.dashboardLayout.glowingBrowserCards); - const browserGlow = glowingBrowserCards[browserId]; + // Only this card's entry: the whole map re-rendered every browser card on every agent action anywhere. + const browserGlow = useAppSelector((s) => s.dashboardLayout.glowingBrowserCards[browserId]); // Drop the glow the moment the agent's done (fading) so it eases off via the 0.4s box-shadow transition, instead of holding full until the entry clears. The tether arrow already keyed off `fading`; the card never did. const showGlow = !!browserGlow && !browserGlow.fading; diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/useCardDrag.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/useCardDrag.ts index 36af7f12..0d8604cf 100644 --- a/frontend/src/app/pages/Dashboard/hooks/interaction/useCardDrag.ts +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/useCardDrag.ts @@ -1,4 +1,5 @@ import { useCallback, useEffect, useRef, useState, type RefObject } from 'react'; +import { useStableCallback } from '@/shared/hooks/useStableCallback'; import { report } from '@/shared/serviceClient'; import { useAppDispatch } from '@/shared/hooks'; import { moveCards } from '@/shared/state/dashboardLayoutSlice'; @@ -86,7 +87,7 @@ export function useCardDrag({ edgePanFrameRef.current = requestAnimationFrame(tickEdgePan); }, [viewportRef, canvasActions]); - const handleCardDragStart = useCallback((id: string, type: CardType) => { + const handleCardDragStartImpl = useCallback((id: string, type: CardType) => { activeDragCardRef.current = id; // Multi only when there is actually company: a lone selected card on this path made every drag after the first pay a setState per frame. if (selection.isSelected(id) && selection.selectedArray().length > 1) { @@ -98,6 +99,7 @@ export function useCardDrag({ isMultiDragRef.current = false; } }, [selection]); + const handleCardDragStart = useStableCallback(handleCardDragStartImpl); const handleCardDragMove = useCallback((dx: number, dy: number, mouseX?: number, mouseY?: number) => { if (mouseX !== undefined && mouseY !== undefined) { @@ -143,7 +145,7 @@ export function useCardDrag({ } }, [stopEdgePan, canvasActions]); - const handleCardDragEnd = useCallback((dx: number, dy: number, didDrag: boolean) => { + const handleCardDragEndImpl = useCallback((dx: number, dy: number, didDrag: boolean) => { if (didDrag) report('dashboard', 'card_dragged'); if (isMultiDragRef.current && didDrag) { const items = selection.selectedArray() @@ -154,6 +156,7 @@ export function useCardDrag({ } clearDrag(); }, [selection, dispatch, clearDrag]); + const handleCardDragEnd = useStableCallback(handleCardDragEndImpl); // Backstop: a pointercancel or a lost pointer capture never reaches the card's onDragEnd, which would otherwise strand the drag with the rAF above panning forever. A normal release runs the card's commit first, since React delegates to the root container and this fires as the event bubbles on past it. useEffect(() => { diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardInteractions.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardInteractions.ts index ee7882ac..95d8ad5e 100644 --- a/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardInteractions.ts +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardInteractions.ts @@ -1,4 +1,5 @@ import React, { useCallback, useEffect, useRef, type Dispatch, type SetStateAction } from 'react'; +import { useStableCallback } from '@/shared/hooks/useStableCallback'; import { report } from '@/shared/serviceClient'; import { useAppDispatch } from '@/shared/hooks'; import { store } from '@/shared/state/store'; @@ -76,7 +77,7 @@ export function useDashboardInteractions({ // Delay single-click collapse so double-click can override const clickTimerRef = useRef | null>(null); - const handleCardSelect = useCallback((id: string, type: CardType, shiftKey: boolean, originTarget?: EventTarget | null) => { + const handleCardSelectImpl = useCallback((id: string, type: CardType, shiftKey: boolean, originTarget?: EventTarget | null) => { report('dashboard', 'card_clicked', { card_type: type, shift: shiftKey }); if (shiftKey) { selection.selectCard(id, type, true); @@ -136,6 +137,7 @@ export function useDashboardInteractions({ }; setTimeout(() => tryFit(0), 100); }, [selection, getCardRect, canvas.actions, dispatch, expandedSessionIds]); + const handleCardSelect = useStableCallback(handleCardSelectImpl); const handleBringToFront = useCallback((id: string, type: CardType) => { // Deferred past the pointerdown's paint: this fires on EVERY card press and the z-restack was diff --git a/frontend/src/app/pages/Dashboard/hooks/lifecycle/useAgentSpawn.ts b/frontend/src/app/pages/Dashboard/hooks/lifecycle/useAgentSpawn.ts index c5ba3d54..08bca26e 100644 --- a/frontend/src/app/pages/Dashboard/hooks/lifecycle/useAgentSpawn.ts +++ b/frontend/src/app/pages/Dashboard/hooks/lifecycle/useAgentSpawn.ts @@ -1,4 +1,5 @@ import { useCallback, type Dispatch, type RefObject, type SetStateAction } from 'react'; +import { useStableCallback } from '@/shared/hooks/useStableCallback'; import { report } from '@/shared/serviceClient'; import { store } from '@/shared/state/store'; import { useAppDispatch } from '@/shared/hooks'; @@ -72,7 +73,7 @@ export function useAgentSpawn({ const dispatch = useAppDispatch(); const getSpawnPlacement = useSpawnPlacement({ selection, viewportRef, canvasStateRef, expandedSessionIds }); - const handleBranchFromCard = useCallback( + const handleBranchFromCardImpl = useCallback( (sourceSessionId: string, newSessionId: string) => { const sourceCard = cards[sourceSessionId]; if (!sourceCard) return; @@ -122,6 +123,7 @@ export function useAgentSpawn({ } setToolbarOpen(true); }, [welcomeEligible, onWelcomeNewAgent, setToolbarOpen]); + const handleBranchFromCard = useStableCallback(handleBranchFromCardImpl); const handleToolbarCancel = useCallback(() => { setToolbarOpen(false); diff --git a/frontend/src/shared/hooks/useStableCallback.ts b/frontend/src/shared/hooks/useStableCallback.ts new file mode 100644 index 00000000..2491e676 --- /dev/null +++ b/frontend/src/shared/hooks/useStableCallback.ts @@ -0,0 +1,12 @@ +import { useCallback, useRef } from 'react'; + +// A callback whose IDENTITY never changes while its body always sees the latest closure. For a +// handler handed to every memoized card on the board: a plain useCallback whose deps include the +// selection re-created itself on every send, and 150 AgentCards re-rendered their whole chrome for +// it (a 380 ms commit on a 4x-throttled machine, the ENG-467 send freeze). Only for event handlers: +// the latest body is read at CALL time, never at render time. +export function useStableCallback(fn: (...args: A) => R): (...args: A) => R { + const latest = useRef(fn); + latest.current = fn; + return useCallback((...args: A) => latest.current(...args), []); +}