From 7dc8c06934c98a559e3a2aa7b78835945ad2a5df Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 14 Aug 2026 10:59:12 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A9=B9=20fix:=20Scope=20phase-entrance=20?= =?UTF-8?q?history=20and=20resolve=20media=20queries=20at=20mount?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both Codex findings on #14832. `MultiMessage` renders siblings without a key, so `ContentParts` survives a sibling switch with its refs intact. The recorded phase-marker set outlived the message it described, and any phase in the newly selected sibling whose index was absent from the previous sibling's set was read as a live arrival — already-loaded history mounted expanded and collapsed itself. Scope the set to its messageId and treat a mismatch as a fresh mount. `useMediaQuery` initialized to `false` and resolved only in a passive effect, so the first render always reported "no match". Anything branching once at mount — the frozen entrance flag here, and every other first-paint decision across its call sites — never saw the correction, which is how a `prefers-reduced-motion: reduce` user still got the fold. Read the query synchronously in the state initializer and guard both paths for environments without `matchMedia`. --- .../Chat/Messages/Content/ContentParts.tsx | 16 +++-- .../Content/__tests__/ContentParts.test.tsx | 37 +++++++++++ .../hooks/__tests__/useMediaQuery.spec.tsx | 66 +++++++++++++++++++ packages/client/src/hooks/useMediaQuery.tsx | 18 ++++- 4 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 packages/client/src/hooks/__tests__/useMediaQuery.spec.tsx diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index 48831bb7d6..6db6f77244 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -234,12 +234,18 @@ const ContentParts = memo(function ContentParts({ /** 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 - * mounted is live; markers present on the first render are history. */ - const previousPhaseIndicesRef = useRef | null>(null); - const previousPhaseIndices = previousPhaseIndicesRef.current; + * mounted is live; markers present on the first render are history. + * + * The recorded set is scoped to the message it described. `MultiMessage` + * renders siblings without a key, so this instance survives a sibling + * switch with its refs intact — an unscoped set would report the previous + * sibling's phases and animate the newly selected sibling's history. */ + const previousPhaseRef = useRef<{ messageId: string; indices: Set } | null>(null); + const previousPhaseIndices = + previousPhaseRef.current?.messageId === messageId ? previousPhaseRef.current.indices : null; useEffect(() => { - previousPhaseIndicesRef.current = completedPhaseIndices; - }, [completedPhaseIndices]); + previousPhaseRef.current = { messageId, indices: completedPhaseIndices }; + }, [messageId, completedPhaseIndices]); const handleGroupExpansionChange = useCallback( (groupId: string, state: ToolCallGroupExpansionState) => { diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx index 206b3b84cc..75cc6977e3 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx @@ -467,4 +467,41 @@ describe('ContentParts — activity phase state', () => { ); expect(screen.getByTestId('tool-call-group')).toHaveAttribute('data-initial-expanded', 'false'); }); + + test('does not replay the entrance when switching to a sibling with its own phases', () => { + const phase = (label: string) => + ({ + type: ContentTypes.ACTIVITY_LABEL, + [ContentTypes.ACTIVITY_LABEL]: label, + activity_label_type: 'phase', + activity_start_index: 0, + activity_count: 1, + pending: false, + }) as unknown as TMessageContentParts; + + const tool = { + type: ContentTypes.TOOL_CALL, + [ContentTypes.TOOL_CALL]: { id: 'tool-1', name: 'search', args: {}, output: 'one' }, + } as unknown as TMessageContentParts; + + const { rerender } = render( + , + ); + expect(screen.getByTestId('activity-phase-group')).toHaveAttribute( + 'data-animate-entrance', + 'false', + ); + + /** MultiMessage swaps siblings without a key, so this instance keeps its + * refs while messageId and content change wholesale. The incoming + * sibling's phase is history and must not animate. */ + rerender( + , + ); + + expect(screen.getByTestId('activity-phase-group')).toHaveAttribute( + 'data-animate-entrance', + 'false', + ); + }); }); diff --git a/packages/client/src/hooks/__tests__/useMediaQuery.spec.tsx b/packages/client/src/hooks/__tests__/useMediaQuery.spec.tsx new file mode 100644 index 0000000000..1f775d3396 --- /dev/null +++ b/packages/client/src/hooks/__tests__/useMediaQuery.spec.tsx @@ -0,0 +1,66 @@ +import { renderHook, act } from '@testing-library/react'; +import useMediaQuery from '../useMediaQuery'; + +type Listener = () => void; + +/** The shared setup installs a writable (not configurable) `matchMedia`, so + * these stubs assign over it rather than redefining the property. */ +function stubMatchMedia(matches: boolean) { + const listeners = new Set(); + const media = { + matches, + addEventListener: (_event: string, listener: Listener) => listeners.add(listener), + removeEventListener: (_event: string, listener: Listener) => listeners.delete(listener), + }; + window.matchMedia = jest.fn(() => media) as unknown as typeof window.matchMedia; + return (next: boolean) => { + media.matches = next; + for (const listener of listeners) { + listener(); + } + }; +} + +describe('useMediaQuery', () => { + const original = window.matchMedia; + + afterEach(() => { + window.matchMedia = original; + }); + + /** Callers that branch once at mount — freezing an entrance animation, + * choosing a layout before paint — only ever see the first render. */ + test('reports a match on the first render, before any effect runs', () => { + stubMatchMedia(true); + + const { result } = renderHook(() => useMediaQuery('(prefers-reduced-motion: reduce)')); + + expect(result.current).toBe(true); + }); + + test('reports no match on the first render when the query does not match', () => { + stubMatchMedia(false); + + const { result } = renderHook(() => useMediaQuery('(prefers-reduced-motion: reduce)')); + + expect(result.current).toBe(false); + }); + + test('tracks later changes to the query', () => { + const change = stubMatchMedia(false); + const { result } = renderHook(() => useMediaQuery('(min-width: 768px)')); + expect(result.current).toBe(false); + + act(() => change(true)); + + expect(result.current).toBe(true); + }); + + test('falls back to no match where matchMedia is unavailable', () => { + window.matchMedia = undefined as unknown as typeof window.matchMedia; + + const { result } = renderHook(() => useMediaQuery('(min-width: 768px)')); + + expect(result.current).toBe(false); + }); +}); diff --git a/packages/client/src/hooks/useMediaQuery.tsx b/packages/client/src/hooks/useMediaQuery.tsx index 067474c964..1a17617776 100644 --- a/packages/client/src/hooks/useMediaQuery.tsx +++ b/packages/client/src/hooks/useMediaQuery.tsx @@ -1,9 +1,25 @@ import { useEffect, useState } from 'react'; +function readMatches(query: string): boolean { + if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') { + return false; + } + return window.matchMedia(query).matches; +} + +/** + * Resolves the query on the FIRST render rather than after a passive effect. + * Callers that branch once at mount — freezing an entrance animation, picking + * a layout before paint — read the deferred value as "no match" and never see + * the correction, which is how `prefers-reduced-motion` came to be ignored. + */ export default function useMediaQuery(query: string): boolean { - const [matches, setMatches] = useState(false); + const [matches, setMatches] = useState(() => readMatches(query)); useEffect(() => { + if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') { + return; + } const media = window.matchMedia(query); if (media.matches !== matches) { setMatches(media.matches);