From 3c084fa01dbb6c840c7b29f6c5ebce79e3377a12 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 14 Aug 2026 10:40:25 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=84=20fix:=20Fold=20activity=20phase?= =?UTF-8?q?=20entrance=20in=20one=20direction,=20flush-left=20label?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phase header replaced with +
+
+ +
+
-
{children}
+ {/** Padding and the divider ride the same curve as the fold: the + * children occupy the exact position they held before the marker + * arrived and settle into the card as it materializes, instead of + * stepping sideways by the card's inset on the first frame. */} +
+ {children} +
diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index 71a67abc27..48831bb7d6 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -215,11 +215,22 @@ const ContentParts = memo(function ContentParts({ (localIndex: number) => contentIndices?.[localIndex] ?? localIndex + contentIndexOffset, [contentIndexOffset, contentIndices], ); - const phaseSegments = nestedActivityPhase ? undefined : groupActivityPhases(content); - const completedPhaseIndices = new Set( - phaseSegments?.flatMap((segment) => (segment.type === 'phase' ? [segment.labelIndex] : [])) ?? - [], + /** Hoisted above the early returns to feed the entrance-detection hook + * below, so it is memoized rather than re-walked on every unrelated + * re-render of a message that has no phases at all. */ + const phaseSegments = useMemo( + () => (nestedActivityPhase ? undefined : groupActivityPhases(content)), + [nestedActivityPhase, content], ); + const completedPhaseIndices = useMemo(() => { + const indices = new Set(); + for (const segment of phaseSegments ?? []) { + if (segment.type === 'phase') { + indices.add(segment.labelIndex); + } + } + return indices; + }, [phaseSegments]); /** A phase label can finish after the root text stream settles, so * `isSubmitting` is not a reliable entrance signal. Compare committed * phase markers instead: a marker that appears after this renderer has @@ -228,7 +239,7 @@ const ContentParts = memo(function ContentParts({ const previousPhaseIndices = previousPhaseIndicesRef.current; useEffect(() => { previousPhaseIndicesRef.current = completedPhaseIndices; - }); + }, [completedPhaseIndices]); const handleGroupExpansionChange = useCallback( (groupId: string, state: ToolCallGroupExpansionState) => { diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index 3e636da8ff..61f9a105d5 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -166,7 +166,7 @@ const Part = memo(function Part({ const failed = part.status === 'failed' || part.status === 'partial'; return (
{display}
diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index d70a10e0bc..85ea2f778e 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -379,7 +379,7 @@ export default function ToolCallGroup({ ({ })); jest.mock('~/hooks', () => { + const expandCollapse = jest.requireActual('~/hooks/Messages/useExpandCollapse'); return { - useExpandCollapse: jest.requireActual('~/hooks/Messages/useExpandCollapse').default, + useExpandCollapse: expandCollapse.default, + EXPAND_TRANSITION: expandCollapse.EXPAND_TRANSITION, scheduleMessageContentLayoutReconcile: (target: HTMLElement | null) => mockScheduleLayoutReconcile(target), }; }); +const LABEL = 'Compared both release paths'; + const labelPart = { type: ContentTypes.ACTIVITY_LABEL, - [ContentTypes.ACTIVITY_LABEL]: 'Compared both release paths', + [ContentTypes.ACTIVITY_LABEL]: LABEL, activity_label_type: 'phase', activity_start_index: 0, pending: false, } as unknown as Extract; describe('ActivityPhaseGroup', () => { - let collapseFrame: FrameRequestCallback | undefined; + let frames: Array; const originalRequestAnimationFrame = window.requestAnimationFrame; const originalCancelAnimationFrame = window.cancelAnimationFrame; + /** The fold waits for a painted start value, so it spans more than one + * frame. Drain the queue the way a real compositor would. */ + const flushFrames = () => + act(() => { + for (let index = 0; index < frames.length; index += 1) { + const frame = frames[index]; + frames[index] = undefined; + frame?.(index); + } + }); + + const pendingFrames = () => frames.filter((frame) => frame != null).length; + beforeEach(() => { - collapseFrame = undefined; + frames = []; mockUseSmoothStreaming.mockReturnValue(true); mockScheduleLayoutReconcile.mockClear(); Object.defineProperty(window, 'requestAnimationFrame', { configurable: true, writable: true, - value: jest.fn((callback: FrameRequestCallback) => { - collapseFrame = callback; - return 1; - }), + value: jest.fn((callback: FrameRequestCallback) => frames.push(callback)), }); Object.defineProperty(window, 'cancelAnimationFrame', { configurable: true, writable: true, - value: jest.fn(), + value: jest.fn((handle: number) => { + frames[handle - 1] = undefined; + }), }); }); @@ -75,21 +91,43 @@ describe('ActivityPhaseGroup', () => { expect(container.querySelector('.result-thinking')).toBeInTheDocument(); }); - test('compresses visible activity into a newly streamed parent label', () => { + test('renders the label flush left with no leading glyph', () => { render( + +
+ , + ); + + const trigger = screen.getByRole('button', { name: LABEL }); + expect(trigger).toHaveClass('justify-start', 'text-left'); + expect(screen.getByText(LABEL)).toHaveClass('flex-1', 'text-left'); + expect(trigger.querySelectorAll('svg')).toHaveLength(1); + }); + + test('mounts in the pre-marker shape, then folds the activity into the summary', () => { + const { container } = render(
, ); - const trigger = screen.getByRole('button', { name: 'Compared both release paths' }); + const card = container.querySelector('.my-2') as HTMLElement; + const trigger = screen.getByRole('button', { name: LABEL }); + const header = trigger.parentElement?.parentElement as HTMLElement; const panel = screen.getByTestId('activity-phase-panel'); + + /** Frame zero must be indistinguishable from the layout the marker + * replaced: no chrome, no header height, activity still open. */ + expect(card).toHaveClass('border-transparent'); + expect(card).not.toHaveClass('bg-surface-secondary/40'); + expect(header).toHaveStyle({ gridTemplateRows: '0fr', opacity: '0' }); expect(trigger).toHaveAttribute('aria-expanded', 'true'); expect(panel).toHaveAttribute('aria-hidden', 'false'); - expect(trigger).toHaveClass('animate-in', 'fade-in-0', 'slide-in-from-bottom-1'); - act(() => collapseFrame?.(0)); + flushFrames(); + expect(header).toHaveStyle({ gridTemplateRows: '1fr', opacity: '1' }); + expect(card).toHaveClass('border-border-light', 'bg-surface-secondary/40'); expect(trigger).toHaveAttribute('aria-expanded', 'false'); expect(panel).toHaveAttribute('aria-hidden', 'true'); expect(mockScheduleLayoutReconcile).toHaveBeenCalledTimes(1); @@ -102,10 +140,11 @@ describe('ActivityPhaseGroup', () => { , ); - const trigger = screen.getByRole('button', { name: 'Compared both release paths' }); + const trigger = screen.getByRole('button', { name: LABEL }); + const header = trigger.parentElement?.parentElement as HTMLElement; expect(trigger).toHaveAttribute('aria-expanded', 'false'); - expect(trigger).not.toHaveClass('animate-in'); - expect(collapseFrame).toBeUndefined(); + expect(header.getAttribute('style')).toBeNull(); + expect(pendingFrames()).toBe(0); fireEvent.click(trigger); expect(trigger).toHaveAttribute('aria-expanded', 'true'); @@ -114,15 +153,43 @@ describe('ActivityPhaseGroup', () => { test('honors the smooth-streaming preference', () => { mockUseSmoothStreaming.mockReturnValue(false); + const { container } = render( + +
+ , + ); + + const trigger = screen.getByRole('button', { name: LABEL }); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + expect(container.querySelector('.my-2')).toHaveClass('border-border-light'); + expect(pendingFrames()).toBe(0); + }); + + test('a click during the entrance wins over the scheduled fold', () => { render(
, ); - const trigger = screen.getByRole('button', { name: 'Compared both release paths' }); + const trigger = screen.getByRole('button', { name: LABEL }); + fireEvent.click(trigger); expect(trigger).toHaveAttribute('aria-expanded', 'false'); - expect(trigger).not.toHaveClass('animate-in'); - expect(collapseFrame).toBeUndefined(); + + flushFrames(); + + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + }); + + test('a phase without activity renders a label-only header', () => { + render( + +
+ , + ); + + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect(screen.getByText(LABEL)).toHaveClass('text-left'); + expect(pendingFrames()).toBe(0); }); }); diff --git a/e2e/specs/mock/activity-phases.spec.ts b/e2e/specs/mock/activity-phases.spec.ts index fe70d04708..23b77f1e4d 100644 --- a/e2e/specs/mock/activity-phases.spec.ts +++ b/e2e/specs/mock/activity-phases.spec.ts @@ -214,7 +214,7 @@ test.describe('parent activity phases', () => { const finalTextIndex = content.findIndex((part) => contentPartText(part).includes(finalText)); expect(finalTextIndex).toBe(phasePart?.activity_end_index); - const parent = messagesView(page).locator(`summary[aria-label="${PARENT_LABEL}"]`); + const parent = messagesView(page).getByRole('button', { name: PARENT_LABEL, exact: true }); await expect(parent).toBeVisible({ timeout: 30000 }); await expect(messagesView(page).getByText(finalText)).toBeVisible({ timeout: 30000 }); await parent.click(); @@ -241,7 +241,10 @@ test.describe('parent activity phases', () => { ).toBe(true); await page.reload(); - const reloadedParent = messagesView(page).locator(`summary[aria-label="${PARENT_LABEL}"]`); + const reloadedParent = messagesView(page).getByRole('button', { + name: PARENT_LABEL, + exact: true, + }); await expect(reloadedParent).toBeVisible({ timeout: 30000 }); await expect(messagesView(page).getByText(finalText)).toBeVisible(); await reloadedParent.click();