From 1704c123949c6448a4dc2b1ac4a999bd565607b9 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 9 Sep 2026 14:13:01 -0400 Subject: [PATCH 1/3] refactor(chat): one module owns the loaded thread window Which response may become the loaded window was arbitrated by six nonces spread across four modules: clearVersion, liveUpdateVersion, windowCleared and windowGateOwner in the thread store, a nextLoadID module global in thread-load, and a load generation the thread-load-status provider handed back UP to the store that owns the loads it was arbitrating. thread-window.tsx now owns it. Callers name a place in the thread - requestWindow({anchor, reason}) with anchor 'newest' | 'older' | 'newer' | {centeredOn} - and read the window back through useThreadWindow(). The gate claim and release, the clear-and-reload, the refusal of a disjoint page, the cached-vs-full pass reconciliation, the back-page retry chain, the load throttle, the scroll repeat gates and the is-this-load-current check are all implementation. clearVersion becomes `generation`, the one nonce: it moves when the window is dropped and when the conversation changes, and both the response gate and the desktop list's dataKey read it. windowGateOwner and nextLoadID become a private gate record in the one module that consults them. ThreadLoadStatusOptions is gone from every call site, and with it the twelve field LoadMoreMessagesParams. liveUpdateVersion stays: it is content churn inside one window rather than a new window, and folding it into the generation would remount the thread list on every incoming message. thread-load-status-context.tsx is deleted. Its ChatThreadsStale and ChatInboxSynced listeners join the other chat notification listeners in thread-engine; its mount-time selectConversation joins the load it issues; useThreadLoadStatus, the one enum load-status.tsx consumes, moves as is. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015rccpV5nLxxC5opF5xzrz7 --- .../chat/conversation/center-context.test.tsx | 45 +- shared/chat/conversation/center-context.tsx | 18 +- shared/chat/conversation/list-area/index.tsx | 82 +- shared/chat/conversation/load-status.tsx | 2 +- .../conversation/normal/container.test.tsx | 6 +- shared/chat/conversation/normal/container.tsx | 6 +- .../chat/conversation/thread-context.test.tsx | 1011 +--------- shared/chat/conversation/thread-context.tsx | 353 +--- shared/chat/conversation/thread-engine.tsx | 31 + .../thread-load-status-context.test.tsx | 114 -- .../thread-load-status-context.tsx | 189 -- shared/chat/conversation/thread-load.test.tsx | 636 +------ shared/chat/conversation/thread-load.tsx | 342 +--- .../chat/conversation/thread-window.test.tsx | 1679 +++++++++++++++++ shared/chat/conversation/thread-window.tsx | 750 ++++++++ 15 files changed, 2599 insertions(+), 2665 deletions(-) delete mode 100644 shared/chat/conversation/thread-load-status-context.test.tsx delete mode 100644 shared/chat/conversation/thread-load-status-context.tsx create mode 100644 shared/chat/conversation/thread-window.test.tsx create mode 100644 shared/chat/conversation/thread-window.tsx diff --git a/shared/chat/conversation/center-context.test.tsx b/shared/chat/conversation/center-context.test.tsx index 9df8cffcf0f8..0829a64ffcff 100644 --- a/shared/chat/conversation/center-context.test.tsx +++ b/shared/chat/conversation/center-context.test.tsx @@ -8,16 +8,12 @@ import {resetAllStores} from '@/util/zustand' const convX = T.Chat.conversationIDToKey(new Uint8Array([1, 2, 3, 4])) const convY = T.Chat.conversationIDToKey(new Uint8Array([5, 6, 7, 8])) -const mockLoadMessagesCentered = jest.fn() -const mockJumpToRecentThread = jest.fn() +const mockRequestWindow = jest.fn() const mockSetMarkReadBlocked = jest.fn() -const mockThreadLoadStatusOptions = {isThreadLoadCurrent: () => true, onThreadLoadStatus: () => {}} let mockRouteParams: {threadSearch?: {query?: string}} | undefined // Both providers under test pull thread/engine plumbing they don't exercise here. jest.mock('./thread-context', () => ({ - useConversationThreadJumpToRecent: () => mockJumpToRecentThread, - useConversationThreadLoadMessagesCentered: () => mockLoadMessagesCentered, useConversationThreadSetMarkReadBlocked: () => mockSetMarkReadBlocked, useConversationThreadStore: () => ({getState: () => ({})}), })) @@ -25,9 +21,7 @@ jest.mock('./send-actions', () => ({ useConversationSendActions: () => ({sendGiphyResult: jest.fn(), sendMessage: jest.fn()}), })) jest.mock('@/engine/action-listener', () => ({useEngineActionListener: () => {}})) -jest.mock('./thread-load-status-context', () => ({ - useThreadLoadStatusOptionsGetter: () => () => mockThreadLoadStatusOptions, -})) +jest.mock('./thread-window', () => ({useRequestWindow: () => mockRequestWindow})) jest.mock('./thread-search-route', () => ({useChatThreadRouteParams: () => mockRouteParams})) import {ConversationCenterProvider, useConversationCenter} from './center-context' @@ -80,29 +74,21 @@ test('a highlight written before mount is consumed on mount', () => { render() expect(mockSetMarkReadBlocked).toHaveBeenCalledWith(true) - expect(mockLoadMessagesCentered).toHaveBeenCalledTimes(1) - expect(mockLoadMessagesCentered).toHaveBeenCalledWith( - T.Chat.numberToMessageID(42), - 'flash', - expect.anything() - ) + expect(mockRequestWindow).toHaveBeenCalledTimes(1) + expect(mockRequestWindow).toHaveBeenCalledWith({anchor: {centeredOn: T.Chat.numberToMessageID(42)}, reason: 'centered'}) expect(seenHighlightOrdinal).toBe(T.Chat.numberToOrdinal(42)) expect(useInputIntentState.getState().intents.has(convX)).toBe(false) }) test('a highlight written after mount is delivered by the subscription', () => { render() - expect(mockLoadMessagesCentered).not.toHaveBeenCalled() + expect(mockRequestWindow).not.toHaveBeenCalled() act(() => { setInputIntent(convX, highlight(7)) }) - expect(mockLoadMessagesCentered).toHaveBeenCalledWith( - T.Chat.numberToMessageID(7), - 'flash', - expect.anything() - ) + expect(mockRequestWindow).toHaveBeenCalledWith({anchor: {centeredOn: T.Chat.numberToMessageID(7)}, reason: 'centered'}) expect(seenHighlightOrdinal).toBe(T.Chat.numberToOrdinal(7)) }) @@ -118,13 +104,8 @@ test('jumping twice to the same message centers both times', () => { setInputIntent(convX, highlight(11)) }) - expect(mockLoadMessagesCentered).toHaveBeenCalledTimes(2) - expect(mockLoadMessagesCentered).toHaveBeenNthCalledWith( - 2, - T.Chat.numberToMessageID(11), - 'flash', - expect.anything() - ) + expect(mockRequestWindow).toHaveBeenCalledTimes(2) + expect(mockRequestWindow).toHaveBeenNthCalledWith(2, {anchor: {centeredOn: T.Chat.numberToMessageID(11)}, reason: 'centered'}) }) // The two-consumer collision the store's `types` filter exists for. @@ -133,11 +114,7 @@ test('the input provider does not consume a highlight meant for the center provi render() - expect(mockLoadMessagesCentered).toHaveBeenCalledWith( - T.Chat.numberToMessageID(5), - 'flash', - expect.anything() - ) + expect(mockRequestWindow).toHaveBeenCalledWith({anchor: {centeredOn: T.Chat.numberToMessageID(5)}, reason: 'centered'}) expect(seenUnsentText).toBeUndefined() }) @@ -147,7 +124,7 @@ test('the center provider does not consume an injectText meant for the input pro render() expect(seenUnsentText).toBe('hello') - expect(mockLoadMessagesCentered).not.toHaveBeenCalled() + expect(mockRequestWindow).not.toHaveBeenCalled() expect(useInputIntentState.getState().intents.has(convX)).toBe(false) }) @@ -156,6 +133,6 @@ test('a highlight for another conversation is left alone', () => { render() - expect(mockLoadMessagesCentered).not.toHaveBeenCalled() + expect(mockRequestWindow).not.toHaveBeenCalled() expect(useInputIntentState.getState().intents.get(convY)).toEqual(highlight(3)) }) diff --git a/shared/chat/conversation/center-context.tsx b/shared/chat/conversation/center-context.tsx index 269a6a8116d7..17dc2ae2945d 100644 --- a/shared/chat/conversation/center-context.tsx +++ b/shared/chat/conversation/center-context.tsx @@ -3,12 +3,8 @@ import * as T from '@/constants/types' import {consumeInputIntent, useInputIntentState} from './input-intent-store' import {produce} from 'immer' import {useChatThreadRouteParams} from './thread-search-route' -import {useThreadLoadStatusOptionsGetter} from './thread-load-status-context' -import { - useConversationThreadJumpToRecent, - useConversationThreadLoadMessagesCentered, - useConversationThreadSetMarkReadBlocked, -} from './thread-context' +import {useConversationThreadSetMarkReadBlocked} from './thread-context' +import {useRequestWindow} from './thread-window' type CenterState = { center: T.Chat.CenterOrdinal | undefined @@ -76,9 +72,7 @@ export const ConversationCenterProvider = function ConversationCenterProvider(p: const {children, id} = p const routeParams = useChatThreadRouteParams() const threadSearchVisible = !!routeParams?.threadSearch - const getThreadLoadStatusOptions = useThreadLoadStatusOptionsGetter() - const loadMessagesCentered = useConversationThreadLoadMessagesCentered() - const jumpToRecentThread = useConversationThreadJumpToRecent() + const requestWindow = useRequestWindow() const setMarkReadBlocked = useConversationThreadSetMarkReadBlocked() const [centerState, setCenterState] = React.useState(() => ({ center: undefined, @@ -109,14 +103,12 @@ export const ConversationCenterProvider = function ConversationCenterProvider(p: const centerOnMessage = (messageID: T.Chat.MessageID, highlightMode: T.Chat.CenterOrdinalHighlightMode) => { setCenterForMessage(messageID, highlightMode) - loadMessagesCentered(messageID, highlightMode, { - ...getThreadLoadStatusOptions(), - }) + requestWindow({anchor: {centeredOn: messageID}, reason: 'centered'}) } const jumpToRecent = () => { clearCenter() - jumpToRecentThread(getThreadLoadStatusOptions()) + requestWindow({anchor: 'newest', reason: 'jump to recent'}) } React.useEffect(() => { diff --git a/shared/chat/conversation/list-area/index.tsx b/shared/chat/conversation/list-area/index.tsx index 7800f4eb543d..baaeeb785f49 100644 --- a/shared/chat/conversation/list-area/index.tsx +++ b/shared/chat/conversation/list-area/index.tsx @@ -14,14 +14,11 @@ import {useConversationCenter} from '../center-context' import { ShownUsernameCacheContext, useConversationThreadID, - useConversationThreadLoadNewerMessagesDueToScroll, - useConversationThreadLoadOlderMessagesDueToScroll, useConversationThreadMarkThreadAsRead, - useConversationThreadSelector, useConversationThreadStore, } from '../thread-context' import {useJumpToRecent} from './jump-to-recent' -import {useThreadLoadStatusOptionsGetter} from '../thread-load-status-context' +import {useRequestWindow, useThreadWindow} from '../thread-window' import {getMessageRowType, getMessageShowUsername} from '../messages/row-metadata' import {useCurrentUserState} from '@/stores/current-user' import * as InputState from '../input-area/input-state' @@ -87,46 +84,17 @@ const useGetItemType = () => { // ==================== SHARED ==================== -// Both platforms read the same slice of thread state. -const useThreadListData = () => - useConversationThreadSelector( - C.useShallow(s => ({ - clearVersion: s.clearVersion, - containsLatestMessage: !s.moreToLoadForward, - loaded: s.loaded, - messageOrdinals: s.messageOrdinals ?? noOrdinals, - })) - ) - -// Pagination: load older at the top of the list, newer at the bottom (only when not already at -// the latest). Refs keep the throttled callbacks stable. -const usePagination = (p: { - containsLatestMessage: boolean - messageOrdinals: ReadonlyArray -}) => { - const {containsLatestMessage, messageOrdinals} = p - const loadOlderMessagesDueToScroll = useConversationThreadLoadOlderMessagesDueToScroll() - const loadNewerMessagesDueToScroll = useConversationThreadLoadNewerMessagesDueToScroll() - const getThreadLoadStatusOptions = useThreadLoadStatusOptionsGetter() - - const numOrdinalsRef = React.useRef(messageOrdinals.length) - React.useEffect(() => { - numOrdinalsRef.current = messageOrdinals.length - }, [messageOrdinals.length]) - - const containsLatestMessageRef = React.useRef(containsLatestMessage) - React.useEffect(() => { - containsLatestMessageRef.current = containsLatestMessage - }, [containsLatestMessage]) +// Pagination: load older at the top of the list, newer at the bottom. Whether either edge has more +// to fetch, and how fast the same edge may re-ask, is thread-window's business. +const usePagination = () => { + const requestWindow = useRequestWindow() const onStartReached = React.useCallback(() => { - loadOlderMessagesDueToScroll(numOrdinalsRef.current, getThreadLoadStatusOptions()) - }, [loadOlderMessagesDueToScroll, getThreadLoadStatusOptions]) + requestWindow({anchor: 'older', reason: 'scroll back'}) + }, [requestWindow]) const onEndReached = C.useThrottledCallback(() => { - if (!containsLatestMessageRef.current) { - loadNewerMessagesDueToScroll(numOrdinalsRef.current, getThreadLoadStatusOptions()) - } + requestWindow({anchor: 'newer', reason: 'scroll forward'}) }, 200) React.useEffect( () => () => { @@ -216,20 +184,20 @@ const DesktopThreadWrapper = function DesktopThreadWrapper() { const desktopStyles = useDesktopStyles() const editingOrdinal = InputState.useConversationInput(s => s.editing) const conversationIDKey = useConversationThreadID() - const data = useThreadListData() + const {generation, loaded, moreToLoadForward, ordinals: messageOrdinals} = useThreadWindow() const {centeredOrdinal} = useConversationCenter() - const {clearVersion, containsLatestMessage, messageOrdinals, loaded} = data + const containsLatestMessage = !moreToLoadForward // Centered loads (search hit, reply-quote jump, pinned message) clear the thread before // refetching, so the list sees a non-empty -> empty -> non-empty transition. - const datasetKey = `${conversationIDKey}:${clearVersion}` + const datasetKey = `${conversationIDKey}:${generation}` const listRef = React.useRef(null) const wrapperRef = React.useRef(null) const markInitiallyLoadedThreadAsRead = useConversationThreadMarkThreadAsRead() - const {onStartReached, onEndReached} = usePagination({containsLatestMessage, messageOrdinals}) + const {onStartReached, onEndReached} = usePagination() // messageOrdinalsRef feeds the imperative scroll-to-center / scroll-to-edit effects below. const messageOrdinalsRef = React.useRef(messageOrdinals) @@ -705,20 +673,15 @@ type RNFlatListRef = { scrollToItem: (opts: {animated: boolean; item: unknown; viewPosition?: number}) => void } -const useInvertedMessageOrdinals = (messageOrdinals?: ReadonlyArray) => { - const source = messageOrdinals ?? noOrdinals - return React.useMemo(() => (source.length > 1 ? [...source].reverse() : source), [source]) -} +const useInvertedMessageOrdinals = (source: ReadonlyArray) => + React.useMemo(() => (source.length > 1 ? [...source].reverse() : source), [source]) const useNativeScrolling = (p: { centeredOrdinal: T.Chat.Ordinal - messageOrdinals: ReadonlyArray listRef: React.RefObject }) => { - const {listRef, centeredOrdinal, messageOrdinals} = p - const numOrdinals = messageOrdinals.length - const loadOlderMessages = useConversationThreadLoadOlderMessagesDueToScroll() - const getThreadLoadStatusOptions = useThreadLoadStatusOptionsGetter() + const {listRef, centeredOrdinal} = p + const requestWindow = useRequestWindow() // KeyboardChatScrollView sets contentInset.top = K - insets.bottom and // contentOffset.y = -(K - insets.bottom) when keyboard is open. Scrolling to @@ -789,7 +752,7 @@ const useNativeScrolling = (p: { }) const onEndReached = () => { - loadOlderMessages(numOrdinals, getThreadLoadStatusOptions()) + requestWindow({anchor: 'older', reason: 'scroll back'}) } return { @@ -829,19 +792,13 @@ const NativeConversationList = function NativeConversationList() { > const conversationIDKey = useConversationThreadID() - const listData = useConversationThreadSelector( - C.useShallow(s => ({ - loaded: s.loaded, - messageOrdinals: s.messageOrdinals, - })) - ) + const {loaded, ordinals} = useThreadWindow() const {centeredHighlightOrdinal, centeredOrdinal} = useConversationCenter() const noCenteredOrdinal = T.Chat.numberToOrdinal(-1) const centeredOrdinalOrNone = centeredOrdinal ?? noCenteredOrdinal const centeredHighlightOrdinalOrNone = centeredHighlightOrdinal ?? noCenteredOrdinal - const {loaded} = listData - const messageOrdinals = useInvertedMessageOrdinals(listData.messageOrdinals) + const messageOrdinals = useInvertedMessageOrdinals(ordinals) const listRef = React.useRef(null) const markInitiallyLoadedThreadAsRead = useConversationThreadMarkThreadAsRead() @@ -886,7 +843,6 @@ const NativeConversationList = function NativeConversationList() { const {scrollToCentered, scrollToBottom, onEndReached, onScrollToIndexFailed} = useNativeScrolling({ centeredOrdinal: centeredOrdinalOrNone, listRef, - messageOrdinals, }) // Closed-loop centering corrector. scrollToItem/scrollToIndex lands at the wrong diff --git a/shared/chat/conversation/load-status.tsx b/shared/chat/conversation/load-status.tsx index 1e2282964322..29f607a306c0 100644 --- a/shared/chat/conversation/load-status.tsx +++ b/shared/chat/conversation/load-status.tsx @@ -2,7 +2,7 @@ import * as React from 'react' import * as Kb from '@/common-adapters' import * as T from '@/constants/types' import logger from '@/logger' -import {useThreadLoadStatus} from './thread-load-status-context' +import {useThreadLoadStatus} from './thread-window' import {useConversationThreadID} from './thread-context' const ValidatedStatus = () => { diff --git a/shared/chat/conversation/normal/container.test.tsx b/shared/chat/conversation/normal/container.test.tsx index 7a2622a60607..3cbf83018a9b 100644 --- a/shared/chat/conversation/normal/container.test.tsx +++ b/shared/chat/conversation/normal/container.test.tsx @@ -64,7 +64,7 @@ function mockPassthroughProvider({children}: {children: React.ReactNode}) { return React.createElement(React.Fragment, null, children) } -function mockConversationThreadLoadStatusProvider( +function mockConversationThreadWindowProvider( props: React.PropsWithChildren<{ allowMarkReadOnLoad?: boolean id: T.Chat.ConversationIDKey @@ -112,8 +112,8 @@ jest.mock('../input-area/input-state', () => { return {ConversationInputProvider: mockPassthroughProvider} }) -jest.mock('../thread-load-status-context', () => { - return {ConversationThreadLoadStatusProvider: mockConversationThreadLoadStatusProvider} +jest.mock('../thread-window', () => { + return {ConversationThreadWindowProvider: mockConversationThreadWindowProvider} }) jest.mock('@/common-adapters/markdown/maybe-mention/context', () => { diff --git a/shared/chat/conversation/normal/container.tsx b/shared/chat/conversation/normal/container.tsx index c1d97028da76..2c4a5aa22d53 100644 --- a/shared/chat/conversation/normal/container.tsx +++ b/shared/chat/conversation/normal/container.tsx @@ -14,7 +14,7 @@ import { useConversationThreadSelector, useThreadMeta, } from '../thread-context' -import {ConversationThreadLoadStatusProvider} from '../thread-load-status-context' +import {ConversationThreadWindowProvider} from '../thread-window' import {MaybeMentionProvider} from '@/common-adapters/markdown/maybe-mention/context' import {peekInputIntent} from '../input-intent-store' import {useChatThreadRouteParams} from '../thread-search-route' @@ -222,13 +222,13 @@ const NormalThreadProviders = ( const {children, id, threadSearchVisible} = p const [pendingHighlight] = React.useState(() => !!peekInputIntent(id, ['highlight'])) return ( - {children} - + ) } diff --git a/shared/chat/conversation/thread-context.test.tsx b/shared/chat/conversation/thread-context.test.tsx index abcf188d74e0..d4af1bebbcfe 100644 --- a/shared/chat/conversation/thread-context.test.tsx +++ b/shared/chat/conversation/thread-context.test.tsx @@ -17,18 +17,13 @@ import { ConversationThreadProvider, LiveConversationThreadProvider, useConversationThreadActions, - useConversationThreadJumpToRecent, - useConversationThreadLoadMoreMessages, - useConversationThreadLoadMessagesCentered, - useConversationThreadLoadOlderMessagesDueToScroll, - useConversationThreadMarkThreadAsRead, useConversationThreadMessage, useConversationThreadMessageActions, useConversationThreadSelector, useConversationThreadStore, } from './thread-context' -import {ConversationThreadLoadStatusProvider} from './thread-load-status-context' import {useConversationParticipants} from './data-hooks' +import {ConversationThreadWindowProvider, useRequestWindow} from './thread-window' const convID = T.Chat.conversationIDToKey(new Uint8Array([1, 2, 3, 4])) const emptyStringSet = new Set() @@ -245,7 +240,18 @@ const separatePlainThreadWrapper = ({children}: {children: React.ReactNode}) => {children} ) +// For the mark-read tests, which need a real thread load to arm mark-read the way the app does. +// The mount-time selection load is skipped so each test issues exactly the load it is about. +const loadingWrapper = ({children}: {children: React.ReactNode}) => ( + + + {children} + + +) + beforeEach(() => { + jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockResolvedValue(undefined) useCurrentUserState.getState().dispatch.setBootstrap({ deviceID: 'device-id', deviceName: 'test-device', @@ -325,183 +331,6 @@ test('mounted thread syncs participant updates received outside its provider', ( expect(result.current.name).toEqual(['alice']) }) -test('centered load clears stale thread state and requests a centered load', async () => { - const loadThread = jest - .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') - .mockResolvedValue({offline: false}) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - loadMessagesCentered: useConversationThreadLoadMessagesCentered(), - messageOrdinals: useConversationThreadSelector(s => s.messageOrdinals), - staleMessage: useConversationThreadMessage(T.Chat.numberToOrdinal(301)), - }), - {wrapper} - ) - - act(() => { - result.current.actions.addMessages([makeTextMessage()]) - }) - expect(result.current.staleMessage?.id).toBe(T.Chat.numberToMessageID(301)) - - act(() => { - result.current.loadMessagesCentered(T.Chat.numberToMessageID(999), 'flash') - }) - await act(async () => { - await flushPromises() - }) - - expect(result.current.staleMessage).toBeUndefined() - expect(result.current.messageOrdinals).toBeUndefined() - expect(loadThread).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - query: expect.objectContaining({ - messageIDControl: expect.objectContaining({ - mode: T.RPCChat.MessageIDControlMode.centered, - pivot: T.Chat.numberToMessageID(999), - }), - }), - }), - }) - ) -}) - -test('jumpToRecent reloads recent messages through the mounted thread action', async () => { - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - const markAsRead = jest - .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') - .mockResolvedValue({offline: false}) - const onThreadLoadStatus = jest.fn() - const msgID = T.Chat.numberToMessageID(202) - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadStatus']?.({ - status: {typ: T.RPCChat.UIChatThreadStatusTyp.server}, - }) - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(msgID, 'recent')], - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook(() => useConversationThreadJumpToRecent(), {wrapper}) - - act(() => { - result.current({onThreadLoadStatus}) - }) - await act(async () => { - await flushPromises() - }) - - expect(onThreadLoadStatus).toHaveBeenCalledWith(convID, T.RPCChat.UIChatThreadStatusTyp.server) - expect(markAsRead).toHaveBeenCalledWith({ - conversationID: T.Chat.keyToConversationID(convID), - forceUnread: false, - msgID, - }) -}) - -test('mark-read disabled latest load does not arm active or explicit mark read', async () => { - useConfigState.setState({loggedIn: true}) - useShellState.getState().dispatch.setActive(false) - jest - .spyOn(Common, 'isUserActivelyLookingAtThisThread') - .mockImplementation(() => useShellState.getState().active) - const markAsRead = jest - .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') - .mockResolvedValue({offline: false}) - const msgID = T.Chat.numberToMessageID(203) - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(msgID, 'search latest')], - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook( - () => ({ - loadMoreMessages: useConversationThreadLoadMoreMessages(), - markThreadAsRead: useConversationThreadMarkThreadAsRead(), - }), - {wrapper} - ) - - act(() => { - result.current.loadMoreMessages({allowMarkAsRead: false, reason: 'focused'}) - }) - await act(async () => { - await flushPromises() - }) - expect(markAsRead).not.toHaveBeenCalled() - - act(() => { - useShellState.getState().dispatch.setActive(true) - }) - await act(async () => { - await flushPromises() - }) - expect(markAsRead).not.toHaveBeenCalled() - - act(() => { - result.current.markThreadAsRead() - }) - await act(async () => { - await flushPromises() - }) - expect(markAsRead).not.toHaveBeenCalled() -}) - -test('scrollback loads older messages without marking the thread read', async () => { - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(T.Chat.numberToMessageID(201), 'older')], - pagination: {last: false, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - return {offline: false} - }) - const markAsRead = jest - .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') - .mockResolvedValue({offline: false}) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - loadOlderMessagesDueToScroll: useConversationThreadLoadOlderMessagesDueToScroll(), - }), - {wrapper} - ) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [makeTextMessage()], - moreToLoad: true, - scrollDirection: 'back', - }) - }) - - act(() => { - result.current.loadOlderMessagesDueToScroll(1) - }) - await act(async () => { - await flushPromises() - }) - - expect(markAsRead).not.toHaveBeenCalled() -}) - test('mounted thread listener applies messagesUpdated for the active conversation', () => { jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) const firstMsgID = T.Chat.numberToMessageID(401) @@ -534,100 +363,7 @@ test('mounted thread listener applies messagesUpdated for the active conversatio expect(result.current.message?.id).toBe(firstMsgID) }) -// The full jump -> scroll-to-bottom -> stale-reload chain behind normal/container.tsx's -// allowMarkReadOnLoad. Jumping to a highlighted message mounts the thread with -// skipThreadLoadOnSelection (the centered load replaces the select-on-mount load) and blocks -// mark-read. The block is NOT permanent: applyThreadLoad releases it as soon as the user scrolls -// to the latest message ('forward' with no moreToLoad). The stale reload that follows - -// ChatThreadsStale fires on every mobile background -> foreground - must then be free to mark the -// thread read. reloadStaleThread reads allowMarkReadOnLoad through useEffectEvent, i.e. the latest -// render's value, so a caller that derived it from the one-shot highlight and froze it at `false` -// would leave the conversation badged unread for as long as the thread stayed mounted. -const staleThreadUpdate = { - payload: { - params: { - uid: '', - updates: [ - {convID: T.Chat.keyToConversationID(convID), updateType: T.RPCChat.StaleUpdateType.newactivity}, - ], - }, - }, - type: 'chat.1.NotifyChat.ChatThreadsStale', -} as never - -const renderJumpedThenScrolledToBottom = (allowMarkReadOnLoad: boolean) => { - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - const markAsRead = jest - .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') - .mockResolvedValue({offline: false}) - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(T.Chat.numberToMessageID(203), 'latest')], - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook(() => useConversationThreadActions(), { - wrapper: ({children}: {children: React.ReactNode}) => ( - - - {children} - - - ), - }) - // jumping to a highlighted message blocks mark-read - act(() => { - result.current.setMarkReadBlocked(true) - }) - // ...then the user scrolls all the way forward to the latest message, releasing the block - act(() => { - result.current.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [makeTextMessage()], - moreToLoad: false, - scrollDirection: 'forward', - }) - }) - return markAsRead -} - -test('a stale reload after a jump and a scroll to the bottom marks the thread read', async () => { - const markAsRead = renderJumpedThenScrolledToBottom(true) - - act(() => { - notifyEngineActionListeners(staleThreadUpdate) - }) - await act(async () => { - await flushPromises() - }) - - expect(markAsRead).toHaveBeenCalledTimes(1) -}) - // The counterfactual: exactly what deriving allowMarkReadOnLoad from the one-shot highlight did. -test('a stale reload that disallows mark read leaves the thread unread even once the block is gone', async () => { - const markAsRead = renderJumpedThenScrolledToBottom(false) - - act(() => { - notifyEngineActionListeners(staleThreadUpdate) - }) - await act(async () => { - await flushPromises() - }) - - expect(markAsRead).not.toHaveBeenCalled() -}) - test('mounted thread listener applies incoming messages for the active conversation', async () => { jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) useConfigState.setState({loggedIn: true}) @@ -648,15 +384,15 @@ test('mounted thread listener applies incoming messages for the active conversat const firstMsgID = T.Chat.numberToMessageID(601) const {result} = renderHook( () => ({ - loadMoreMessages: useConversationThreadLoadMoreMessages(), message: useConversationThreadMessage(T.Chat.numberToOrdinal(601)), ordinals: useConversationThreadSelector(s => s.messageOrdinals), + requestWindow: useRequestWindow(), }), - {wrapper} + {wrapper: loadingWrapper} ) act(() => { - result.current.loadMoreMessages({reason: 'focused'}) + result.current.requestWindow({anchor: 'newest', reason: 'focused'}) }) await act(async () => { await flushPromises() @@ -767,10 +503,10 @@ test('an unlocalized conversation defers mark read until localization lands', as await Promise.resolve() return {offline: false} }) - const {result} = renderHook(() => useConversationThreadLoadMoreMessages(), {wrapper}) + const {result} = renderHook(useRequestWindow, {wrapper: loadingWrapper}) act(() => { - result.current({reason: 'tab selected'}) + result.current({anchor: 'newest', reason: 'tab selected'}) }) await act(async () => { await flushPromises() @@ -821,10 +557,10 @@ test('active change marks read after an eligible mounted thread load', async () await Promise.resolve() return {offline: false} }) - const {result} = renderHook(() => useConversationThreadLoadMoreMessages(), {wrapper}) + const {result} = renderHook(useRequestWindow, {wrapper: loadingWrapper}) act(() => { - result.current({reason: 'tab selected'}) + result.current({anchor: 'newest', reason: 'tab selected'}) }) await act(async () => { await flushPromises() @@ -864,10 +600,10 @@ test('active change does not mark read after a centered thread load', async () = await Promise.resolve() return {offline: false} }) - const {result} = renderHook(() => useConversationThreadLoadMessagesCentered(), {wrapper}) + const {result} = renderHook(useRequestWindow, {wrapper: loadingWrapper}) act(() => { - result.current(msgID, 'flash') + result.current({anchor: {centeredOn: msgID}, reason: 'centered'}) }) await act(async () => { await flushPromises() @@ -1006,14 +742,14 @@ test('mounted thread listener applies reaction updates for the active conversati }) const {result} = renderHook( () => ({ - loadMoreMessages: useConversationThreadLoadMoreMessages(), message: useConversationThreadMessage(T.Chat.numberToOrdinal(301)), + requestWindow: useRequestWindow(), }), - {wrapper} + {wrapper: loadingWrapper} ) act(() => { - result.current.loadMoreMessages({reason: 'focused'}) + result.current.requestWindow({anchor: 'newest', reason: 'focused'}) }) await act(async () => { await flushPromises() @@ -1066,22 +802,25 @@ test('mounted thread listener applies reaction updates for the active conversati }) }) -test('loaded focus refresh does not overwrite newer streamed reaction updates', async () => { +test('toggleMessageReaction overlays locally without mutating server reactions', async () => { const targetMsgID = T.Chat.numberToMessageID(301) const targetOrdinal = T.Chat.numberToOrdinal(301) - let incomingCallMap: - | Parameters[0]['incomingCallMap'] - | undefined - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - incomingCallMap = p.incomingCallMap - await Promise.resolve() - return {offline: false} - }) + const postReaction = jest + .spyOn(T.RPCChat, 'localPostReactionNonblockRpcPromise') + .mockImplementation(async p => { + const outboxID = await Promise.resolve(p.outboxID ?? new Uint8Array()) + return { + identifyFailures: null, + outboxID, + rateLimits: null, + } + }) const {result} = renderHook( () => ({ actions: useConversationThreadActions(), - loadMoreMessages: useConversationThreadLoadMoreMessages(), message: useConversationThreadMessage(targetOrdinal), + messageActions: useConversationThreadMessageActions(), + store: useConversationThreadStore(), }), {wrapper} ) @@ -1089,7 +828,7 @@ test('loaded focus refresh does not overwrite newer streamed reaction updates', act(() => { result.current.actions.applyThreadLoad({ centered: false, - enableActiveMarkRead: true, + enableActiveMarkRead: false, messages: [makeTextMessage()], moreToLoad: false, scrollDirection: 'none', @@ -1097,13 +836,41 @@ test('loaded focus refresh does not overwrite newer streamed reaction updates', }) act(() => { - result.current.loadMoreMessages({reason: 'tab selected'}) + result.current.messageActions.toggleMessageReaction(targetOrdinal, ':+1:') }) + + expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) + expect(result.current.store.getState().messageMap.get(targetOrdinal)?.reactions).toBeUndefined() await act(async () => { await flushPromises() }) - expect(incomingCallMap).toBeDefined() + const outboxID = postReaction.mock.calls[0]?.[0].outboxID + expect(outboxID).toBeDefined() + + act(() => { + notifyEngineActionListeners({ + payload: { + params: { + activity: { + activityType: T.RPCChat.ChatActivityType.incomingMessage, + incomingMessage: makeIncomingOutboxReaction( + convID, + outboxID!, + targetMsgID, + ':+1:', + 'decorated-plus-one' + ), + }, + }, + }, + type: 'chat.1.NotifyChat.NewChatActivity', + } as never) + }) + + expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) + expect(result.current.message?.reactions?.get(':+1:')?.decorated).toBe('decorated-plus-one') + expect(result.current.store.getState().messageMap.get(targetOrdinal)?.reactions).toBeUndefined() act(() => { notifyEngineActionListeners({ @@ -1118,7 +885,7 @@ test('loaded focus refresh does not overwrite newer streamed reaction updates', reactions: { reactions: { ':+1:': { - decorated: ':+1:', + decorated: 'server-plus-one', users: { alice: { ctime: 300, @@ -1140,129 +907,10 @@ test('loaded focus refresh does not overwrite newer streamed reaction updates', } as never) }) - expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) - - act(() => { - incomingCallMap?.['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(targetMsgID, 'stale server copy')], - pagination: {last: true, next: '', num: 20, previous: ''}, - }), - }) - }) - - expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) -}) - -test('toggleMessageReaction overlays locally without mutating server reactions', async () => { - const targetMsgID = T.Chat.numberToMessageID(301) - const targetOrdinal = T.Chat.numberToOrdinal(301) - const postReaction = jest - .spyOn(T.RPCChat, 'localPostReactionNonblockRpcPromise') - .mockImplementation(async p => { - const outboxID = await Promise.resolve(p.outboxID ?? new Uint8Array()) - return { - identifyFailures: null, - outboxID, - rateLimits: null, - } - }) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - message: useConversationThreadMessage(targetOrdinal), - messageActions: useConversationThreadMessageActions(), - store: useConversationThreadStore(), - }), - {wrapper} - ) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [makeTextMessage()], - moreToLoad: false, - scrollDirection: 'none', - }) - }) - - act(() => { - result.current.messageActions.toggleMessageReaction(targetOrdinal, ':+1:') - }) - - expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) - expect(result.current.store.getState().messageMap.get(targetOrdinal)?.reactions).toBeUndefined() - await act(async () => { - await flushPromises() - }) - - const outboxID = postReaction.mock.calls[0]?.[0].outboxID - expect(outboxID).toBeDefined() - - act(() => { - notifyEngineActionListeners({ - payload: { - params: { - activity: { - activityType: T.RPCChat.ChatActivityType.incomingMessage, - incomingMessage: makeIncomingOutboxReaction( - convID, - outboxID!, - targetMsgID, - ':+1:', - 'decorated-plus-one' - ), - }, - }, - }, - type: 'chat.1.NotifyChat.NewChatActivity', - } as never) - }) - - expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) - expect(result.current.message?.reactions?.get(':+1:')?.decorated).toBe('decorated-plus-one') - expect(result.current.store.getState().messageMap.get(targetOrdinal)?.reactions).toBeUndefined() - - act(() => { - notifyEngineActionListeners({ - payload: { - params: { - activity: { - activityType: T.RPCChat.ChatActivityType.reactionUpdate, - reactionUpdate: { - convID: T.Chat.keyToConversationID(convID), - reactionUpdates: [ - { - reactions: { - reactions: { - ':+1:': { - decorated: 'server-plus-one', - users: { - alice: { - ctime: 300, - reactionMsgID: T.Chat.messageIDToNumber(T.Chat.numberToMessageID(99)), - }, - }, - }, - }, - }, - targetMsgID: T.Chat.messageIDToNumber(targetMsgID), - }, - ], - userReacjis: {skinTone: T.RPCGen.ReacjiSkinTone.none, topReacjis: null}, - }, - }, - }, - }, - type: 'chat.1.NotifyChat.NewChatActivity', - } as never) - }) - - expect(result.current.store.getState().optimisticReactionMap.size).toBe(0) - expect(result.current.store.getState().messageMap.get(targetOrdinal)?.reactions?.get(':+1:')).toEqual({ - decorated: 'server-plus-one', - users: [{timestamp: 300, username: 'alice'}], + expect(result.current.store.getState().optimisticReactionMap.size).toBe(0) + expect(result.current.store.getState().messageMap.get(targetOrdinal)?.reactions?.get(':+1:')).toEqual({ + decorated: 'server-plus-one', + users: [{timestamp: 300, username: 'alice'}], }) }) @@ -1466,292 +1114,6 @@ test('mounted thread listener applies attachment download and upload progress', ).toBeUndefined() }) -test('a warm-cache load prunes against both passes, not either one alone', async () => { - // Regression: once the service has sent a cached thread it switches the full response to - // INCREMENTAL, so the full pass only carries what changed. Treating either pass on its own as - // authoritative deleted real messages that were still in the thread. The two together are the - // window - INCREMENTAL walks it and omits only what the cached pass already carried - so the - // range spans both, and everything inside it that either pass carried survives. - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) - const ids = [301, 302, 303, 304].map(T.Chat.numberToMessageID) - const threadJSON = (msgIDs: ReadonlyArray) => - JSON.stringify({ - messages: msgIDs.map(id => makeValidTextUIMessage(id, `m${id}`)), - pagination: {last: true, next: '', num: 100, previous: ''}, - }) - - // The cache holds the older three; only 304 changed, so that is all the full pass carries. The - // span is what makes this the dangerous shape: a range of [301..304] computed from the full pass - // alone covers 302 and 303, which are absent from it and would be pruned. - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({thread: threadJSON(ids.slice(0, 3))}) - await Promise.resolve() - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({thread: threadJSON([ids[3]!])}) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - loadMoreMessages: useConversationThreadLoadMoreMessages(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - // Seed a settled four-message window the way a whole-window full pass would. - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: ids.map(id => - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id, - ordinal: T.Chat.numberToOrdinal(T.Chat.messageIDToNumber(id)), - outboxID: undefined, - text: new HiddenString(`m${id}`), - timestamp: 100, - }) - ), - moreToLoad: false, - scrollDirection: 'none', - }) - }) - expect(result.current.ordinals).toEqual([301, 302, 303, 304]) - - act(() => { - result.current.loadMoreMessages({reason: 'test'}) - }) - await act(async () => { - await flushPromises() - }) - - expect(result.current.ordinals).toEqual([301, 302, 303, 304]) -}) - -test('a warm-cache load does not prune a message sitting on its outbox ordinal', async () => { - // A message you sent keeps the fractional ordinal it had in the outbox, so the ordinal it parses - // with - its server one - is not the ordinal it occupies. The prune walks the window, so what - // the passes delivered has to be recorded in the window's terms too; recording the parsed - // ordinal deletes the row it was meant to protect. - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) - const outboxID = T.Chat.stringToOutboxID('sent-1') - const sentOrdinal = T.Chat.numberToOrdinal(302.001) - - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({ - thread: JSON.stringify({ - messages: [ - makeValidTextUIMessage(T.Chat.numberToMessageID(301), 'm301'), - makeValidTextUIMessage(T.Chat.numberToMessageID(302), 'm302'), - makeValidTextUIMessage(T.Chat.numberToMessageID(303), 'mine', 'sent-1'), - ], - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(T.Chat.numberToMessageID(301), 'm301 edited')], - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - loadMoreMessages: useConversationThreadLoadMoreMessages(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - // The window as it stands after the send settled: the message is at its outbox ordinal, indexed - // under the server ID the service will send it back as. - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [ - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id: T.Chat.numberToMessageID(301), - ordinal: T.Chat.numberToOrdinal(301), - outboxID: undefined, - text: new HiddenString('m301'), - timestamp: 100, - }), - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id: T.Chat.numberToMessageID(302), - ordinal: T.Chat.numberToOrdinal(302), - outboxID: undefined, - text: new HiddenString('m302'), - timestamp: 100, - }), - Message.makeMessageText({ - author: 'testuser', - conversationIDKey: convID, - id: T.Chat.numberToMessageID(303), - ordinal: sentOrdinal, - outboxID, - text: new HiddenString('mine'), - timestamp: 100, - }), - ], - moreToLoad: false, - scrollDirection: 'none', - }) - }) - expect(result.current.ordinals).toEqual([301, 302, sentOrdinal]) - - act(() => { - result.current.loadMoreMessages({reason: 'test'}) - }) - await act(async () => { - await flushPromises() - }) - - expect(result.current.ordinals).toEqual([301, 302, sentOrdinal]) -}) - -test('a full pass that changed nothing still reconciles the window', async () => { - // The ordinary warm reload: the cached pass is the window and the INCREMENTAL full pass behind it - // carries nothing at all, because nothing changed. That is still an authoritative answer about - // the span, so a row the service no longer has is still a ghost - skipping the prune for want of - // messages to add leaves it on screen until the conversation is reopened. - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) - const ids = [301, 302, 303].map(T.Chat.numberToMessageID) - - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({ - thread: JSON.stringify({ - messages: [ids[0]!, ids[2]!].map(id => makeValidTextUIMessage(id, `m${id}`)), - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({messages: null, pagination: {last: true, next: '', num: 100, previous: ''}}), - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - loadMoreMessages: useConversationThreadLoadMoreMessages(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: ids.map(id => - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id, - ordinal: T.Chat.numberToOrdinal(T.Chat.messageIDToNumber(id)), - outboxID: undefined, - text: new HiddenString(`m${id}`), - timestamp: 100, - }) - ), - moreToLoad: false, - scrollDirection: 'none', - }) - }) - expect(result.current.ordinals).toEqual([301, 302, 303]) - - act(() => { - result.current.loadMoreMessages({reason: 'test'}) - }) - await act(async () => { - await flushPromises() - }) - - expect(result.current.ordinals).toEqual([301, 303]) -}) - -test('a warm-cache load still prunes a row neither pass carries', async () => { - // The other half of the same rule: a row inside the range that neither pass returned is a ghost - - // a cache repair left it behind, or it was deleted while we were away - and reconciling it away - // is what the range is for. Gating on a full pass with no cached one before it would have given - // this up for every conversation the cache is warm for. - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) - const ids = [301, 302, 303, 304].map(T.Chat.numberToMessageID) - const threadJSON = (msgIDs: ReadonlyArray) => - JSON.stringify({ - messages: msgIDs.map(id => makeValidTextUIMessage(id, `m${id}`)), - pagination: {last: true, next: '', num: 100, previous: ''}, - }) - - // 303 is in neither pass, and it sits inside the span the two of them cover. - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({thread: threadJSON([ids[0]!, ids[1]!])}) - await Promise.resolve() - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({thread: threadJSON([ids[3]!])}) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - loadMoreMessages: useConversationThreadLoadMoreMessages(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: ids.map(id => - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id, - ordinal: T.Chat.numberToOrdinal(T.Chat.messageIDToNumber(id)), - outboxID: undefined, - text: new HiddenString(`m${id}`), - timestamp: 100, - }) - ), - moreToLoad: false, - scrollDirection: 'none', - }) - }) - expect(result.current.ordinals).toEqual([301, 302, 303, 304]) - - act(() => { - result.current.loadMoreMessages({reason: 'test'}) - }) - await act(async () => { - await flushPromises() - }) - - expect(result.current.ordinals).toEqual([301, 302, 304]) -}) - // The window invariant, at the callsite that enforces it. The four unit tests in // thread-message-state.test.tsx pass `dropNewBelowWindow` themselves; only this proves addMessages // actually sets it, and that thread loads are still allowed to extend the window downward. @@ -1803,189 +1165,6 @@ test('a notification may not strand a new ordinal below the loaded window', () = ) }) -test('jumpToRecent drops the old window instead of merging a disjoint one into it', async () => { - // The newest window has nothing to do with wherever the reader had scrolled back to, so merging - // the two leaves a hole in messageOrdinals between them - which is the same stranded-index-0 - // shape that kills scrollback. A centered jump already clears first; this must too. - useConfigState.setState({loggedIn: true}) - jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) - jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) - const recent = T.Chat.numberToMessageID(9001) - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ - thread: JSON.stringify({ - messages: [makeValidTextUIMessage(recent, 'newest')], - pagination: {last: true, next: '', num: 100, previous: ''}, - }), - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - jumpToRecent: useConversationThreadJumpToRecent(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - // The reader is deep in old history. - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [textAt(101), textAt(102)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - - act(() => { - result.current.jumpToRecent() - }) - await act(async () => { - await flushPromises() - }) - - // Only the newest window survives. If the old one were merged in, ordinals would read - // [101, 102, 9001] with a 8899-wide hole. - expect(result.current.ordinals).toEqual([T.Chat.numberToOrdinal(9001)]) -}) - -test('only the load that claimed the window gate may drop it', () => { - // clearVersion cannot separate two loads of the same conversation - the load generation only - // moves on a conversation change or unmount, so both call themselves current. The reader taps a - // search result, messagesClear issues the centered reload, and a ChatThreadsStale notification - // then fires a second load at the same generation. If that one settles first - no thread, an - // error - it would take the gate down while the reload is still in flight, and a push landing in - // what is left of the gap strands exactly as it did before the gate existed. - const {result} = renderHook(() => ({actions: useConversationThreadActions()}), {wrapper}) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [textAt(7152), textAt(7153)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - act(() => { - result.current.actions.messagesClear() - }) - - // The reload the clear issued claims the gate; the stale-thread load that follows loses the race. - act(() => { - result.current.actions.claimWindowGate(1) - result.current.actions.claimWindowGate(2) - }) - act(() => { - result.current.actions.clearWindowGate(2) - }) - expect(result.current.actions.getSnapshot().windowCleared).toBe(true) - - act(() => { - result.current.actions.clearWindowGate(1) - }) - expect(result.current.actions.getSnapshot().windowCleared).toBe(false) -}) - -test('a stale reload does not merge the newest page into a centered window', () => { - // The reader taps a search result and sits on the window around it, with more to load forward. - // A ChatThreadsStale reload fetches the newest page, which is nowhere near that window: merging - // the two leaves ordinals with a hole through the middle and then calls the result the latest - // message, which is the gap this invariant is about. - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - const textAt = (ord: number) => - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id: T.Chat.numberToMessageID(ord), - ordinal: T.Chat.numberToOrdinal(ord), - outboxID: undefined, - text: new HiddenString(`m${ord}`), - timestamp: 100, - }) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: true, - enableActiveMarkRead: false, - messages: [textAt(7000), textAt(7001)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - expect(result.current.actions.getSnapshot().moreToLoadForward).toBe(true) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [textAt(9900), textAt(9901)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - - expect(result.current.ordinals).toEqual([7000, 7001]) - // ...and the window still knows it has not reached the latest message. - expect(result.current.actions.getSnapshot().moreToLoadForward).toBe(true) -}) - -test('a newest page that reaches the window is still merged', () => { - // The other side of the rule. A reader near the bottom gets a page that overlaps what they hold, - // so there is no hole to open and the refresh must land. - const {result} = renderHook( - () => ({ - actions: useConversationThreadActions(), - ordinals: useConversationThreadSelector(s => s.messageOrdinals), - }), - {wrapper} - ) - - const textAt = (ord: number) => - Message.makeMessageText({ - author: 'alice', - conversationIDKey: convID, - id: T.Chat.numberToMessageID(ord), - ordinal: T.Chat.numberToOrdinal(ord), - outboxID: undefined, - text: new HiddenString(`m${ord}`), - timestamp: 100, - }) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: true, - enableActiveMarkRead: false, - messages: [textAt(9900), textAt(9901)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [textAt(9901), textAt(9902)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - - expect(result.current.ordinals).toEqual([9900, 9901, 9902]) -}) - test('an empty pass leaves the thread unloaded rather than loaded and empty', () => { // addMessagesToThreadState always leaves a messageOrdinals array behind, and the top-of-thread // block reads `messageOrdinals !== undefined` as "this conversation has loaded at least once". @@ -2006,50 +1185,6 @@ test('an empty pass leaves the thread unloaded rather than loaded and empty', () expect(result.current.actions.getSnapshot().messageOrdinals).toBeUndefined() }) -test('an empty pass during a jump-to-recent gap leaves the gate up', () => { - // A cold cache sends a cached pass carrying no messages ahead of the full response, and it - // reaches applyThreadLoad like any other. Dropping the gate on it reopens the gap: a - // notification landing before the real page becomes the sole ordinal, and the page that follows - // is disjoint from it. - const {result} = renderHook(() => ({actions: useConversationThreadActions()}), {wrapper}) - - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [textAt(7152), textAt(7153)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - act(() => { - result.current.actions.messagesClear() - }) - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - act(() => { - result.current.actions.addMessages([textAt(7155)], {liveUpdate: true}) - }) - act(() => { - result.current.actions.applyThreadLoad({ - centered: false, - enableActiveMarkRead: false, - messages: [textAt(9001)], - moreToLoad: true, - scrollDirection: 'none', - }) - }) - - expect(result.current.actions.getSnapshot().messageOrdinals).toEqual([T.Chat.numberToOrdinal(9001)]) -}) - test('a notification during a jump-to-recent gap cannot become the new window', () => { // jumpToRecent and a centered jump both clear before reloading, so for one RPC round trip the // window is empty. The notification most likely to land in that gap is the post-send diff --git a/shared/chat/conversation/thread-context.tsx b/shared/chat/conversation/thread-context.tsx index 0a7abbe52b08..259c11638072 100644 --- a/shared/chat/conversation/thread-context.tsx +++ b/shared/chat/conversation/thread-context.tsx @@ -6,13 +6,10 @@ import * as T from '@/constants/types' import {getVisibleScreen, navigateAppend, navigateToThread, navigateUp, setChatRootParams} from '@/constants/router' import {isPhone} from '@/constants/platform' import logger from '@/logger' -import throttle from 'lodash/throttle' -import {clearChatTimeCache} from '@/util/timestamp' import {findLast} from '@/util/arrays' import {ignorePromise} from '@/constants/utils' import {RPCError} from '@/util/errors' import {useCurrentUserState} from '@/stores/current-user' -import {useUsersState} from '@/stores/users' import {useConfigState} from '@/stores/config' import {useShellState} from '@/stores/shell' import {produce, type Draft} from 'immer' @@ -40,13 +37,7 @@ import { updateAttachmentUploadProgressInThreadState, updateReactionsInThreadState, } from './thread-message-state' -import { - getInboxConversationMeta, - getInboxConversationParticipants, - metasReceived, - unboxRows, - useInboxMetadataState, -} from '@/chat/inbox/metadata' +import {getInboxConversationMeta, metasReceived, useInboxMetadataState} from '@/chat/inbox/metadata' import {loadThreadMessageIDAtIndex, markConversationRead} from './thread-rpc' import { cancelConversationPost, @@ -61,9 +52,6 @@ import { getClientPrevFromSnapshot, getExplodingModeFromConfig, getMeta, - loadConversationThreadMessages, - numMessagesOnInitialLoad, - numMessagesOnScrollback, persistExplodingMode, } from './thread-load' import {useThreadEngineListeners} from './thread-engine' @@ -80,12 +68,6 @@ const sameStringSet = (a: ReadonlySet, b: ReadonlySet) => { return true } -const emptyParticipantInfo: T.Chat.ParticipantInfo = { - all: [], - contactName: new Map(), - name: [], -} - const formatTextForQuoting = (text: string) => text .split('\n') @@ -97,10 +79,12 @@ ConversationThreadIDContext.displayName = 'ConversationThreadIDContext' export type ConversationThreadState = { accountsInfoMap: Map - // Bumped on every messagesClear. The desktop list remounts on it: LegendList cannot recover + // The identity of the loaded window: bumped whenever the window is dropped (messagesClear) or the + // conversation under it changes. Two things read it. thread-window refuses any response fetched + // against an older generation, and the desktop list remounts on it - LegendList cannot recover // from a non-empty -> empty -> non-empty data transition (it resets its layout state and waits // for a container layout event that never comes), so the thread renders blank forever. - clearVersion: number + generation: number explodingMode: number flipStatusMap: Map loaded: boolean @@ -109,15 +93,9 @@ export type ConversationThreadState = { messageMap: Map messageOrdinals?: ReadonlyArray // Set between a messagesClear and the reload that refills the window, so a notification arriving - // in that gap cannot install itself as the new window. Cleared once that load settles, however it - // settles - see clearWindowGate. + // in that gap cannot install itself as the new window. thread-window decides which load may take + // it down; see releaseWindowGate. windowCleared?: boolean - // The load that owns the gate above: the first one to claim it after the clear, which is the - // reload the clear issued. Only that load may drop the gate. clearVersion alone cannot tell two - // loads of the same conversation apart, and a second load at the same generation - a - // ChatThreadsStale reload, say - would otherwise settle first and take down a gate the reload is - // still relying on. - windowGateOwner?: number messageTypeMap: Map moreToLoadBack: boolean moreToLoadForward: boolean @@ -144,7 +122,7 @@ const makeEmptyThreadState = (): ConversationThreadState => produce( { accountsInfoMap: new Map(), - clearVersion: 0, + generation: 0, explodingMode: 0, flipStatusMap: new Map(), liveUpdateVersion: 0, @@ -173,54 +151,7 @@ const makeInitialThreadState = (id: T.Chat.ConversationIDKey) => { const makeThreadStore = (id: T.Chat.ConversationIDKey) => createStore(() => makeInitialThreadState(id)) -export type ThreadLoadStatusOptions = { - isThreadLoadCurrent?: () => boolean - onThreadLoadStatus?: ThreadLoadStatusReporter -} - -type SelectedConversationOptions = ThreadLoadStatusOptions & { - allowMarkAsRead?: boolean - skipThreadLoad?: boolean -} - export type ScrollDirection = 'none' | 'back' | 'forward' -export type LoadMoreMessagesParams = ThreadLoadStatusOptions & { - allowMarkAsRead?: boolean - centeredMessageID?: { - conversationIDKey: T.Chat.ConversationIDKey - highlightMode: T.Chat.CenterOrdinalHighlightMode - messageID: T.Chat.MessageID - } - forceContainsLatestCalc?: boolean - knownRemotes?: ReadonlyArray - messageIDControl?: T.RPCChat.MessageIDControl | null - numberOfMessagesToLoad?: number - reason: string - // Internal: set only by the empty-back-page reload in thread-load.tsx, carrying the oldest - // message ID the previous attempt saw. Each reload must reach strictly further back than that, - // which is what stops it looping. Callers leave it unset. - retryBelowMessageID?: T.Chat.MessageID - // How many times the back-page reload has already chained. See maxBackPageReloads. - retryCount?: number - scrollDirection?: ScrollDirection -} -type LoadMoreMessages = ((p: LoadMoreMessagesParams) => void) & {cancel: () => void} -type LoadMessagesCentered = ( - messageID: T.Chat.MessageID, - highlightMode: T.Chat.CenterOrdinalHighlightMode, - options?: ThreadLoadStatusOptions -) => void -type LoadOlderMessagesDueToScroll = ( - numOrdinals: number, - options?: ThreadLoadStatusOptions -) => void -type LoadNewerMessagesDueToScroll = ( - numOrdinals: number, - options?: ThreadLoadStatusOptions -) => void -type JumpToRecent = (options?: ThreadLoadStatusOptions) => void -type MessagesClear = () => void -type SelectedConversation = (options?: SelectedConversationOptions) => void export type ConversationThreadActions = { addMessages: ( messages: ReadonlyArray, @@ -233,12 +164,12 @@ export type ConversationThreadActions = { centered: boolean disableActiveMarkRead?: boolean enableActiveMarkRead: boolean - forceContainsLatestCalc?: boolean messages: ReadonlyArray moreToLoad: boolean reconcile?: ThreadLoadReconcile scrollDirection: ScrollDirection }) => void + bumpWindowGeneration: () => void clearUnfurlPrompt: (messageID: T.Chat.MessageID, domain: string) => void deleteMessages: (p: { messageIDs?: ReadonlyArray @@ -252,17 +183,15 @@ export type ConversationThreadActions = { explodedBy?: string, liveUpdate?: boolean ) => void - claimWindowGate: (loadID: number) => void - clearWindowGate: (loadID: number) => void getSnapshot: () => ConversationThreadState - loadMoreMessages: LoadMoreMessages markThreadAsRead: () => void setMarkReadBlocked: (blocked: boolean) => void messageDelete: (ordinal: T.Chat.Ordinal) => void messageReplyPrivately: (ordinal: T.Chat.Ordinal) => void - messagesClear: MessagesClear + messagesClear: () => void receivePaymentInfo: (messageID: T.Chat.MessageID, paymentInfo: T.Chat.ChatPaymentInfo) => void receiveRequestInfo: (messageID: T.Chat.MessageID, requestInfo: T.Chat.ChatRequestInfo) => void + releaseWindowGate: () => void retryMessage: (outboxID: T.Chat.OutboxID) => void setExplodingMode: (seconds: number, incoming?: boolean) => void setMessageErrored: (outboxID: T.Chat.OutboxID, reason: string, errorTyp?: number) => void @@ -298,11 +227,6 @@ const ConversationThreadActionsContext = React.createContext void - export const useConversationThreadID = () => { const conversationIDKey = React.useContext(ConversationThreadIDContext) if (!conversationIDKey) { @@ -319,26 +243,6 @@ export const useConversationThreadActions = () => { return actions } -const useScrollLoadGate = () => { - const lastScrollNumOrdinalsRef = React.useRef(0) - const lastScrollTimeRef = React.useRef(0) - return (numOrdinals: number) => { - const now = Date.now() - if (numOrdinals !== lastScrollNumOrdinalsRef.current) { - lastScrollNumOrdinalsRef.current = numOrdinals - lastScrollTimeRef.current = now - return true - } - - const ok = now - lastScrollTimeRef.current > 500 - if (ok) { - lastScrollNumOrdinalsRef.current = numOrdinals - lastScrollTimeRef.current = now - } - return ok - } -} - export const useConversationThreadSelector = ( selector: (snapshot: ConversationThreadState) => TValue ) => { @@ -535,48 +439,11 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => centered: boolean disableActiveMarkRead?: boolean enableActiveMarkRead: boolean - forceContainsLatestCalc?: boolean messages: ReadonlyArray moreToLoad: boolean reconcile?: ThreadLoadReconcile scrollDirection: ScrollDirection }) => { - const rendered = p.messages.filter(m => m.conversationMessage !== false && m.type !== 'deleted') - // Judged on what this pass carried rather than on the state of the window, so the gate turns - // on the one thing that decides it: whether this pass put a row on screen. - const carriedRenderedMessage = rendered.length > 0 - // A 'none' load fetches the newest page, and a window with more to load forward does not - // reach it. Merging the two leaves ordinals with a hole through the middle, and the branch - // below then reports that window as containing the latest message - which is the gap this - // whole invariant is about, arriving through a ChatThreadsStale reload while the reader sits - // on a search result. Both conditions are needed: a window that already reaches the newest - // message merges fine, and so does a page that overlaps what we hold, however far back the - // reader is. Neither holds here, so the page is left alone rather than applied - the reader - // keeps their window, and jumping to recent (which empties it first) is what replaces it. - const beforeApply = threadStore.getState() - const windowOrdinals = beforeApply.messageOrdinals - const floor = windowOrdinals?.[0] - const ceiling = windowOrdinals?.[windowOrdinals.length - 1] - if ( - p.scrollDirection === 'none' && - rendered.length && - beforeApply.moreToLoadForward && - floor !== undefined && - ceiling !== undefined - ) { - let lowest = Number.MAX_SAFE_INTEGER - let highest = Number.MIN_SAFE_INTEGER - for (const m of rendered) { - lowest = Math.min(lowest, m.ordinal) - highest = Math.max(highest, m.ordinal) - } - if (lowest > ceiling || highest < floor) { - logger.info( - `applyThreadLoad: page ${lowest}-${highest} does not reach window ${floor}-${ceiling}, ignoring` - ) - return - } - } updateThreadState(s => { s.loaded = true // The reconciling pass runs even with nothing to add: the warm reload where nothing changed @@ -589,15 +456,6 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => addMessagesToThreadState(s, p.messages, {reconcile: p.reconcile}) clearOptimisticReactionsForMessagesInThreadState(s, p.messages) } - // Only a pass that actually rendered something drops the gate. A cold cache sends an empty - // cached pass ahead of the full response, and a page can be all tombstones: dropping the - // gate on either would let a notification arriving before the real page install itself as - // the whole window and strand once that page lands. A load that ends without ever producing - // an ordinal releases the gate in its own finally instead - see clearWindowGate. - if (carriedRenderedMessage) { - s.windowCleared = false - s.windowGateOwner = undefined - } switch (p.scrollDirection) { case 'forward': s.moreToLoadForward = p.moreToLoad @@ -610,7 +468,7 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => // A centered window may already include the latest message; leaving // moreToLoadForward true would drop live incoming messages and block mark-read. let containsLatest = false - if (p.centered && p.forceContainsLatestCalc) { + if (p.centered) { const {maxVisibleMsgID} = getMeta(id) const ordinal = findLast(s.messageOrdinals ?? [], o => !!s.messageMap.get(o)?.id) const message = ordinal ? s.messageMap.get(ordinal) : undefined @@ -962,41 +820,22 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => markThreadAsRead() } ) - // The reload a clear issues claims the gate, so a load that merely happens to be running at the - // same clear generation cannot drop it out from under that reload. First claim wins: the clear - // issues its reload synchronously, so that reload is the first to get here. - const claimWindowGate = React.useEffectEvent((loadID: number) => { - const s = threadStore.getState() - if (!s.windowCleared || s.windowGateOwner !== undefined) { - return - } + // Which load may take the gate down is thread-window's decision; this only performs it. + const releaseWindowGate = React.useEffectEvent(() => { updateThreadState(d => { - d.windowGateOwner = loadID + d.windowCleared = false }) }) - // applyThreadLoad drops the gate when a load refills the window, but a load can end without ever - // applying: offline, scchatnotinteam, or a response that carries no thread. Left alone the gate - // would keep dropping notifications for the life of the provider, with no window to correct it. - const clearWindowGate = React.useEffectEvent((loadID: number) => { - const s = threadStore.getState() - if (!s.windowCleared) { - return - } - // An unclaimed gate is released by whoever settles first: nothing claimed it, so there is no - // reload in flight to protect, and leaving it up would strand the thread. - if (s.windowGateOwner !== undefined && s.windowGateOwner !== loadID) { - return - } + const bumpWindowGeneration = React.useEffectEvent(() => { updateThreadState(d => { - d.windowCleared = false - d.windowGateOwner = undefined + d.generation += 1 }) }) const messagesClear = React.useEffectEvent(() => { activeMarkReadEnabledRef.current = false shownUsernameCache.clear() updateThreadState(s => { - s.clearVersion += 1 + s.generation += 1 s.pendingOutboxToOrdinal.clear() s.loaded = false // Mark the gap. A notification landing between here and the reload would otherwise face an @@ -1005,7 +844,6 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => // arbitrary one, jumpToRecent the newest page - so nothing arriving first can be placed // against what is coming. s.windowCleared = true - s.windowGateOwner = undefined s.messageIDToOrdinal.clear() s.messageMap.clear() s.messageOrdinals = undefined @@ -1103,46 +941,25 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => } ) const [threadActions] = React.useState(() => { - const impl = (p: LoadMoreMessagesParams) => loadConversationThreadMessages(id, p, threadActions) - const throttled = throttle(impl, 500) - // The throttle keeps only the last trailing call, so a centered or jump-to-recent - // load issued between two other loads would be silently dropped — after - // loadMessagesCentered already cleared the thread. Run those immediately instead. - const loadMoreMessages: LoadMoreMessages = Object.assign( - (p: LoadMoreMessagesParams) => { - if (p.centeredMessageID || p.messageIDControl || p.reason === 'jump to recent') { - throttled.cancel() - impl(p) - } else { - throttled(p) - } - }, - { - cancel: () => { - throttled.cancel() - }, - } - ) const threadActions: ConversationThreadActions = { addMessages, addOptimisticReaction, applyThreadLoad, + bumpWindowGeneration, clearUnfurlPrompt, - claimWindowGate, - clearWindowGate, completeAttachmentDownload, deleteMessages, explodeMessages, failAttachmentDownload, finishAttachmentDownload, getSnapshot, - loadMoreMessages, markThreadAsRead, messageDelete, messageReplyPrivately, messagesClear, receivePaymentInfo, receiveRequestInfo, + releaseWindowGate, removeOptimisticReaction, retryMessage, setAttachmentMobileSaving, @@ -1165,11 +982,6 @@ const ConversationThreadProviderInner = (p: ConversationThreadProviderProps) => } return threadActions }) - React.useEffect(() => { - return () => { - threadActions.loadMoreMessages.cancel() - } - }, [threadActions]) useThreadEngineListeners(id, threadActions) return ( @@ -1229,103 +1041,6 @@ export const getConversationThreadDisplayMessage = ( export const useConversationThreadMessage = (ordinal: T.Chat.Ordinal) => useConversationThreadSelector(snapshot => getConversationThreadDisplayMessage(snapshot, ordinal)) -export const useConversationThreadLoadMoreMessages = () => useConversationThreadActions().loadMoreMessages - -const useConversationThreadMessagesClear = () => useConversationThreadActions().messagesClear - -export const useConversationThreadLoadOlderMessagesDueToScroll = () => { - const threadStore = useConversationThreadStore() - const loadMoreMessages = useConversationThreadLoadMoreMessages() - const okToLoadMore = useScrollLoadGate() - - const loadOlderMessagesDueToScroll: LoadOlderMessagesDueToScroll = (numOrdinals, options) => { - if (!threadStore.getState().moreToLoadBack) { - logger.info('bail: scrolling back and at the end') - return - } - - if (!numOrdinals) { - return - } - - if (!okToLoadMore(numOrdinals)) { - return - } - - loadMoreMessages({ - ...(options ?? {}), - numberOfMessagesToLoad: numMessagesOnScrollback, - reason: 'scroll back', - scrollDirection: 'back', - }) - } - return loadOlderMessagesDueToScroll -} - -export const useConversationThreadLoadNewerMessagesDueToScroll = () => { - const loadMoreMessages = useConversationThreadLoadMoreMessages() - const okToLoadMore = useScrollLoadGate() - - const loadNewerMessagesDueToScroll: LoadNewerMessagesDueToScroll = (numOrdinals, options) => { - if (!numOrdinals) { - return - } - - if (!okToLoadMore(numOrdinals)) { - return - } - - loadMoreMessages({ - ...(options ?? {}), - numberOfMessagesToLoad: numMessagesOnScrollback, - reason: 'scroll forward', - scrollDirection: 'forward', - }) - } - return loadNewerMessagesDueToScroll -} - -export const useConversationThreadLoadMessagesCentered = () => { - const conversationIDKey = useConversationThreadID() - const loadMoreMessages = useConversationThreadLoadMoreMessages() - const messagesClear = useConversationThreadMessagesClear() - - const loadMessagesCentered: LoadMessagesCentered = (messageID, highlightMode, options) => { - messagesClear() - loadMoreMessages({ - centeredMessageID: { - conversationIDKey, - highlightMode, - messageID, - }, - forceContainsLatestCalc: true, - messageIDControl: { - mode: T.RPCChat.MessageIDControlMode.centered, - num: numMessagesOnInitialLoad, - pivot: messageID, - }, - ...(options ?? {}), - reason: 'centered', - }) - } - return loadMessagesCentered -} - -export const useConversationThreadJumpToRecent = () => { - const {setMarkReadBlocked} = useConversationThreadActions() - const loadMoreMessages = useConversationThreadLoadMoreMessages() - const messagesClear = useConversationThreadMessagesClear() - - const jumpToRecent: JumpToRecent = options => { - setMarkReadBlocked(false) - // The newest window is disjoint from wherever the reader was, so merging the two would leave a - // gap in the ordinals. Drop the old window first, the way a centered jump does. - messagesClear() - loadMoreMessages({...(options ?? {}), reason: 'jump to recent'}) - } - return jumpToRecent -} - export const useConversationThreadMarkThreadAsRead = () => useConversationThreadActions().markThreadAsRead export const useConversationThreadSetMarkAsUnread = () => useConversationThreadActions().setMarkAsUnread @@ -1338,34 +1053,6 @@ export const useConversationThreadMessageActions = () => { return {messageDelete, messageReplyPrivately, toggleMessageCollapse, toggleMessageReaction, unfurlRemove} } -export const useConversationThreadSelectedConversation = () => { - const conversationIDKey = useConversationThreadID() - const loadMoreMessages = useConversationThreadLoadMoreMessages() - - const selectedConversation: SelectedConversation = (options?: SelectedConversationOptions) => { - const {skipThreadLoad, ...loadStatusOptions} = options ?? {} - clearChatTimeCache() - - unboxRows([conversationIDKey]) - - const username = useCurrentUserState.getState().username - const participantInfo = getInboxConversationParticipants(conversationIDKey) ?? emptyParticipantInfo - const otherParticipants = Meta.getRowParticipants(participantInfo, username || '') - if (otherParticipants.length === 1) { - const otherUsername = otherParticipants[0] || '' - - if (otherUsername && !otherUsername.includes('@')) { - useUsersState.getState().dispatch.getBio(otherUsername) - } - } - - if (!skipThreadLoad) { - loadMoreMessages({...loadStatusOptions, reason: 'focused'}) - } - } - return selectedConversation -} - export const useConversationThreadToggleSearch = () => { const conversationIDKey = useConversationThreadID() return (hide?: boolean, query?: string) => { diff --git a/shared/chat/conversation/thread-engine.tsx b/shared/chat/conversation/thread-engine.tsx index d8fed1eaf88f..e47611e5b96d 100644 --- a/shared/chat/conversation/thread-engine.tsx +++ b/shared/chat/conversation/thread-engine.tsx @@ -228,6 +228,37 @@ export const applyEphemeralPurgeToThread = ( } } +// The stale-thread notifications, kept with the rest of the chat notification listeners rather +// than in the module that reacts to them. Both mean the same thing - the window we hold may no +// longer match the service - and the reaction is the caller's: thread-window reloads the newest +// page. +export const useThreadStaleReloadListeners = ( + id: T.Chat.ConversationIDKey, + reloadStaleThread: () => void +): void => { + useEngineActionListener('chat.1.NotifyChat.ChatThreadsStale', action => { + const hasStaleThread = (action.payload.params.updates ?? []).some( + update => T.Chat.conversationIDToKey(update.convID) === id + ) + if (hasStaleThread) { + reloadStaleThread() + } + }) + + useEngineActionListener('chat.1.NotifyChat.ChatInboxSynced', action => { + const {syncRes} = action.payload.params + if (syncRes.syncType !== T.RPCChat.SyncInboxResType.incremental) { + return + } + const hasStaleThread = (syncRes.incremental.items ?? []).some( + item => T.Chat.stringToConversationIDKey(item.conv.convID) === id + ) + if (hasStaleThread) { + reloadStaleThread() + } + }) +} + export const useThreadEngineListeners = ( id: T.Chat.ConversationIDKey, threadActions: ConversationThreadActions diff --git a/shared/chat/conversation/thread-load-status-context.test.tsx b/shared/chat/conversation/thread-load-status-context.test.tsx deleted file mode 100644 index 797ac75ffebc..000000000000 --- a/shared/chat/conversation/thread-load-status-context.test.tsx +++ /dev/null @@ -1,114 +0,0 @@ -/** @jest-environment jsdom */ -/// -import {act, cleanup, renderHook} from '@testing-library/react' -import type * as React from 'react' -import * as T from '@/constants/types' -import {notifyEngineActionListeners} from '@/engine/action-listener' -import {resetAllStores} from '@/util/zustand' -import {useCurrentUserState} from '@/stores/current-user' -import { - ConversationThreadLoadStatusProvider, - useThreadLoadStatus, - useThreadLoadStatusOptions, - useThreadLoadStatusReporter, -} from './thread-load-status-context' -import {ConversationThreadProvider} from './thread-context' - -const convID = T.Chat.conversationIDToKey(new Uint8Array([1, 2, 3, 4])) -const otherConvID = T.Chat.conversationIDToKey(new Uint8Array([5, 6, 7, 8])) - -const flushPromises = async () => { - for (let i = 0; i < 5; i++) { - await Promise.resolve() - } -} - -beforeEach(() => { - jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockResolvedValue(undefined) - useCurrentUserState.getState().dispatch.setBootstrap({ - deviceID: 'device-id', - deviceName: 'test-device', - uid: 'uid', - username: 'alice', - }) -}) - -afterEach(() => { - cleanup() - jest.restoreAllMocks() - resetAllStores() -}) - -const wrapper = ({children}: {children: React.ReactNode}) => ( - - - {children} - - -) - -test('thread load status reporter ignores stale conversation statuses', () => { - const {result} = renderHook( - () => ({ - report: useThreadLoadStatusReporter(), - status: useThreadLoadStatus(), - }), - {wrapper} - ) - - expect(result.current.status).toBe(T.RPCChat.UIChatThreadStatusTyp.none) - - act(() => { - result.current.report(otherConvID, T.RPCChat.UIChatThreadStatusTyp.server) - }) - expect(result.current.status).toBe(T.RPCChat.UIChatThreadStatusTyp.none) - - act(() => { - result.current.report(convID, T.RPCChat.UIChatThreadStatusTyp.server) - }) - expect(result.current.status).toBe(T.RPCChat.UIChatThreadStatusTyp.server) -}) - -test('thread load options invalidate when the mounted provider unmounts', () => { - const {result, unmount} = renderHook(() => useThreadLoadStatusOptions(), {wrapper}) - const options = result.current - - expect(options.isThreadLoadCurrent?.()).toBe(true) - - unmount() - - expect(options.isThreadLoadCurrent?.()).toBe(false) -}) - -test('mounted stale-thread reload reports status through the provider', async () => { - jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { - p.incomingCallMap['chat.1.chatUi.chatThreadStatus']?.({ - status: {typ: T.RPCChat.UIChatThreadStatusTyp.server}, - }) - await Promise.resolve() - return {offline: false} - }) - const {result} = renderHook(() => useThreadLoadStatus(), {wrapper}) - - act(() => { - notifyEngineActionListeners({ - payload: { - params: { - uid: '', - updates: [ - { - convID: T.Chat.keyToConversationID(convID), - updateType: T.RPCChat.StaleUpdateType.newactivity, - }, - ], - }, - }, - type: 'chat.1.NotifyChat.ChatThreadsStale', - } as never) - }) - await act(async () => { - await flushPromises() - }) - - expect(result.current).toBe(T.RPCChat.UIChatThreadStatusTyp.server) -}) diff --git a/shared/chat/conversation/thread-load-status-context.tsx b/shared/chat/conversation/thread-load-status-context.tsx deleted file mode 100644 index 6b5475628268..000000000000 --- a/shared/chat/conversation/thread-load-status-context.tsx +++ /dev/null @@ -1,189 +0,0 @@ -import * as React from 'react' -import * as T from '@/constants/types' -import {useEngineActionListener} from '@/engine/action-listener' -import logger from '@/logger' -import { - type ThreadLoadStatusOptions, - type ThreadLoadStatusReporter, - useConversationThreadLoadMoreMessages, - useConversationThreadSelectedConversation, -} from './thread-context' - -type ThreadLoadStatusState = { - conversationIDKey: T.Chat.ConversationIDKey - status: T.RPCChat.UIChatThreadStatusTyp -} - -type ThreadLoadStatusOptionsCache = { - generation: number - options: ThreadLoadStatusOptions -} - -type ThreadLoadStatusStatusContextType = T.RPCChat.UIChatThreadStatusTyp - -type ThreadLoadStatusActionContextType = { - getThreadLoadStatusOptions: () => ThreadLoadStatusOptions - onThreadLoadStatus: ThreadLoadStatusReporter -} - -const noThreadLoadStatus = T.RPCChat.UIChatThreadStatusTyp.none -const ignoreThreadLoadStatus: ThreadLoadStatusReporter = () => {} -const ignoreThreadLoadStatusOptions = () => ({ - isThreadLoadCurrent: () => true, - onThreadLoadStatus: ignoreThreadLoadStatus, -}) - -const missingThreadLoadStatusContext = () => { - throw new Error('Missing ConversationThreadLoadStatusProvider in the tree') -} - -const missingThreadLoadStatusOptions = () => { - missingThreadLoadStatusContext() - return ignoreThreadLoadStatusOptions() -} - -const ThreadLoadStatusContext = React.createContext(noThreadLoadStatus) -ThreadLoadStatusContext.displayName = 'ThreadLoadStatusContext' - -const ThreadLoadStatusActionContext = React.createContext({ - getThreadLoadStatusOptions: missingThreadLoadStatusOptions, - onThreadLoadStatus: missingThreadLoadStatusContext, -}) -ThreadLoadStatusActionContext.displayName = 'ThreadLoadStatusActionContext' - -export const useThreadLoadStatus = () => React.useContext(ThreadLoadStatusContext) - -export const useThreadLoadStatusReporter = () => - React.useContext(ThreadLoadStatusActionContext).onThreadLoadStatus - -export const useThreadLoadStatusOptions = () => - React.useContext(ThreadLoadStatusActionContext).getThreadLoadStatusOptions() - -export const useThreadLoadStatusOptionsGetter = () => - React.useContext(ThreadLoadStatusActionContext).getThreadLoadStatusOptions - -export const ConversationThreadLoadStatusProvider = ( - p: React.PropsWithChildren<{ - allowMarkReadOnLoad?: boolean - id: T.Chat.ConversationIDKey - skipThreadLoadOnSelection?: boolean - }> -) => { - const {allowMarkReadOnLoad = true, children, id, skipThreadLoadOnSelection = false} = p - const [initialSkipThreadLoadOnSelection] = React.useState(skipThreadLoadOnSelection) - const loadMoreMessages = useConversationThreadLoadMoreMessages() - const selectedConversation = useConversationThreadSelectedConversation() - const currentIDRef = React.useRef(id) - React.useLayoutEffect(() => { - currentIDRef.current = id - }, [id]) - const threadLoadGenerationRef = React.useRef(0) - const mountedRef = React.useRef(true) - const threadLoadStatusOptionsRef = React.useRef(undefined) - const [threadLoadStatusState, setThreadLoadStatusState] = React.useState(() => ({ - conversationIDKey: id, - status: noThreadLoadStatus, - })) - const [threadLoadStatusActions] = React.useState(() => { - const onThreadLoadStatus: ThreadLoadStatusReporter = (conversationIDKey, status) => { - if (conversationIDKey !== currentIDRef.current) { - return - } - setThreadLoadStatusState(previous => - previous.conversationIDKey === conversationIDKey && previous.status === status - ? previous - : {conversationIDKey, status} - ) - } - - const getThreadLoadStatusOptions = (): ThreadLoadStatusOptions => { - const generation = threadLoadGenerationRef.current - const cached = threadLoadStatusOptionsRef.current - if (cached?.generation === generation) { - return cached.options - } - const options = { - isThreadLoadCurrent: () => mountedRef.current && threadLoadGenerationRef.current === generation, - onThreadLoadStatus, - } - threadLoadStatusOptionsRef.current = {generation, options} - return options - } - - return {getThreadLoadStatusOptions, onThreadLoadStatus} - }) - const {getThreadLoadStatusOptions} = threadLoadStatusActions - - React.useEffect(() => { - mountedRef.current = true - return () => { - mountedRef.current = false - } - }, []) - - React.useEffect(() => { - return () => { - // Only invalidate generation when the conversation changes. In React StrictMode, - // effects run twice (mount → cleanup → remount) with the same id — incrementing here - // would discard the first RPC's callbacks as stale while the daemon deduplicates - // the second RPC and sends no data. - if (currentIDRef.current !== id) { - threadLoadGenerationRef.current += 1 - } - } - }, [id]) - - const status = - threadLoadStatusState.conversationIDKey === id ? threadLoadStatusState.status : noThreadLoadStatus - - const reloadStaleThread = () => { - loadMoreMessages({ - allowMarkAsRead: allowMarkReadOnLoad, - ...getThreadLoadStatusOptions(), - reason: 'got stale', - }) - } - - useEngineActionListener('chat.1.NotifyChat.ChatThreadsStale', action => { - const hasStaleThread = (action.payload.params.updates ?? []).some( - update => T.Chat.conversationIDToKey(update.convID) === id - ) - if (hasStaleThread) { - reloadStaleThread() - } - }) - - useEngineActionListener('chat.1.NotifyChat.ChatInboxSynced', action => { - const {syncRes} = action.payload.params - if (syncRes.syncType !== T.RPCChat.SyncInboxResType.incremental) { - return - } - const hasStaleThread = (syncRes.incremental.items ?? []).some( - item => T.Chat.stringToConversationIDKey(item.conv.convID) === id - ) - if (hasStaleThread) { - reloadStaleThread() - } - }) - - const selectConversation = React.useEffectEvent(() => { - selectedConversation({ - allowMarkAsRead: allowMarkReadOnLoad, - ...getThreadLoadStatusOptions(), - skipThreadLoad: initialSkipThreadLoadOnSelection, - }) - }) - - React.useEffect(() => { - logger.info( - `ConversationThreadLoadStatusProvider: selecting thread: ${id} skipThreadLoad=${initialSkipThreadLoadOnSelection}` - ) - selectConversation() - }, [id, initialSkipThreadLoadOnSelection]) - - return ( - - {children} - - ) -} diff --git a/shared/chat/conversation/thread-load.test.tsx b/shared/chat/conversation/thread-load.test.tsx index 993a38961ce2..0d5ad57c0e3b 100644 --- a/shared/chat/conversation/thread-load.test.tsx +++ b/shared/chat/conversation/thread-load.test.tsx @@ -6,20 +6,9 @@ import { getExplodingModeFromGregorItems, getLastOrdinalFromSnapshot, getOrdinalForMessageIDInSnapshot, - loadConversationThreadMessages, - maxBackPageReloads, - numMessagesOnScrollback, scrollDirectionToPagination, } from './thread-load' -import * as ThreadRpc from './thread-rpc' -import {resetAllStores} from '@/util/zustand' -import {useCurrentUserState} from '@/stores/current-user' -import type {ThreadLoadReconcile} from './thread-message-state' -import type { - ConversationThreadActions, - ConversationThreadState, - LoadMoreMessagesParams, -} from './thread-context' +import type {ConversationThreadState} from './thread-context' const conversationIDKey = T.Chat.stringToConversationIDKey('conv1') const otherConversationIDKey = T.Chat.stringToConversationIDKey('conv2') @@ -166,626 +155,3 @@ describe('snapshot helpers', () => { expect(getOrdinalForMessageIDInSnapshot(snapshot, messageID(7))).toBeNull() }) }) - -describe('a back page that adds no ordinals reloads itself', () => { - const flushPromises = async () => { - for (let i = 0; i < 200; i++) { - await Promise.resolve() - } - } - - // A stand-in for the real store that keeps the one behaviour under test: applying a load adds an - // ordinal per message EXCEPT the ones addMessagesToThreadState drops, which is `deleted`. A fake - // that grows unconditionally would make every page look productive and hide the bug; one that - // never grows would make every page look empty and hide the opposite bug. - const trackingActions = () => { - const ordinals = new Set([ - T.Chat.numberToOrdinal(7152), - T.Chat.numberToOrdinal(7153), - ]) - const actions = { - applyThreadLoad: jest.fn((p: {messages: ReadonlyArray}) => { - for (const m of p.messages) { - if (m.type !== 'deleted') { - ordinals.add(m.ordinal) - } - } - }), - claimWindowGate: jest.fn(), - clearWindowGate: jest.fn(), - getSnapshot: () => - ({ - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: [...ordinals].sort((a, b) => a - b), - pendingOutboxToOrdinal: new Map(), - }) as unknown as ConversationThreadState, - // The reload goes through the action, which in the store is the throttled loadMoreMessages. - // Standing in the unthrottled call here keeps these tests about the reload chain rather than - // about lodash timers; the throttle itself is store wiring. - loadMoreMessages: jest.fn((p: LoadMoreMessagesParams) => { - loadConversationThreadMessages(conversationIDKey, p, actions) - }), - markThreadAsRead: jest.fn(), - } as unknown as ConversationThreadActions - return actions - } - - // Hidden placeholders are what a DELETE-superseded message arrives as, and what becomes `deleted` - // on this side. They carry real message IDs, which is what bounds the reload. - const tombstones = (from: number, to: number) => - Array.from({length: from - to + 1}, (_, i) => ({ - placeholder: {hidden: true, messageID: T.Chat.numberToMessageID(from - i)}, - state: T.RPCChat.MessageUnboxedState.placeholder, - })) - - // hidden: false parses to a `placeholder`, which the thread does render and keep an ordinal for. - const visible = (from: number, to: number) => - Array.from({length: from - to + 1}, (_, i) => ({ - placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(from - i)}, - state: T.RPCChat.MessageUnboxedState.placeholder, - })) - - // Each call walks one page further back, exactly as the service does, until it runs out. - const mockWalkingBack = (oldestOverall: number) => { - let next = 7151 - return jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - const from = next - const to = Math.max(oldestOverall, from - numMessagesOnScrollback + 1) - next = to - 1 - await Promise.resolve() - p.onFullThread?.( - JSON.stringify({messages: tombstones(from, to), pagination: {last: to <= oldestOverall, num: 100}}) - ) - return undefined as never - }) - } - - const loadBack = (actions: ConversationThreadActions) => - loadConversationThreadMessages( - conversationIDKey, - {numberOfMessagesToLoad: numMessagesOnScrollback, reason: 'scroll back', scrollDirection: 'back'}, - actions - ) - - beforeEach(() => { - useCurrentUserState.getState().dispatch.setBootstrap({ - deviceID: 'device-id', - deviceName: 'testuser-mac', - uid: 'uid', - username: 'testuser', - }) - }) - - afterEach(() => { - jest.restoreAllMocks() - resetAllStores() - }) - - test('keeps paging through a run of tombstones until the pager says it is done', async () => { - // 7151 down to 6952 is two pages of 100, so one reload after the first call. - const rpc = mockWalkingBack(6952) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(2) - }) - - test('walks a run of tombstones that ends before the cap', async () => { - const oldest = 6752 - const rpc = mockWalkingBack(oldest) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(Math.ceil((7151 - oldest + 1) / numMessagesOnScrollback)) - }) - - test('stops at the reload cap rather than walking an expunged history', async () => { - // A channel whose history was largely expunged has far more tombstones than the chain should - // walk off one gesture. It stops at the cap and hands the thread back; scrolling away and back - // fires onStartReached again and starts a fresh chain from where this one stopped. - const rpc = mockWalkingBack(1) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(maxBackPageReloads + 1) - }) - - test('stops if a page fails to reach further back', async () => { - // A service that keeps handing back the same window must not spin us forever. Progress in - // message ID is the only thing permitting another attempt. - const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onFullThread?.( - JSON.stringify({messages: tombstones(7151, 7052), pagination: {last: false, num: 100}}) - ) - return undefined as never - }) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(2) - }) - - test('does not reload when the page actually added ordinals', async () => { - // Renderable messages, so the store grows and the list will ask for the next page itself. - const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onFullThread?.( - JSON.stringify({messages: visible(7151, 7052), pagination: {last: false, num: 100}}) - ) - return undefined as never - }) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(1) - }) - - test('reloads when a warm cache delivers the tombstones', async () => { - // The reported bug's own shape: the conversation is already in local storage, so PullLocalOnly - // wins and the cached pass carries the page - which is entirely tombstones. Judging only the - // full pass, or refusing to judge at all once a cached pass arrived, leaves this inert. - let next = 7151 - const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - const from = next - const to = Math.max(6952, from - numMessagesOnScrollback + 1) - next = to - 1 - await Promise.resolve() - const body = JSON.stringify({ - messages: tombstones(from, to), - pagination: {last: to <= 6952, num: 100}, - }) - p.onCachedThread?.(body) - // The full pass is INCREMENTAL once a cached thread has been sent. - p.onFullThread?.(JSON.stringify({messages: tombstones(to, to), pagination: {last: to <= 6952, num: 100}})) - return undefined as never - }) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(2) - }) - - test('does not reload after a cached pass already delivered the page', async () => { - // The normal warm-cache sequence: PullLocalOnly wins, the cached pass carries the whole page, - // and the full pass that follows is INCREMENTAL - only the messages that changed, every one of - // them already in the window. On ordinal count alone that is indistinguishable from a page of - // tombstones, and reloading on it walks the client back through the entire conversation. - const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onCachedThread?.( - JSON.stringify({messages: visible(7151, 7052), pagination: {last: false, num: 100}}) - ) - p.onFullThread?.( - JSON.stringify({messages: visible(7052, 7052), pagination: {last: false, num: 100}}) - ) - return undefined as never - }) - loadBack(trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(1) - }) - - test('stops when the window is cleared under it', async () => { - // jump to recent and a centered jump both clear then reload. A chain still walking backwards - // would prepend pages into a window the reader has just left, producing the disjoint ordinals - // this whole branch exists to prevent. - let calls = 0 - const ordinals = new Set([T.Chat.numberToOrdinal(7152)]) - let clearVersion = 0 - const actions = { - applyThreadLoad: jest.fn(), - claimWindowGate: jest.fn(), - getSnapshot: () => - ({ - clearVersion, - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: [...ordinals].sort((a, b) => a - b), - pendingOutboxToOrdinal: new Map(), - }) as unknown as ConversationThreadState, - markThreadAsRead: jest.fn(), - } as unknown as ConversationThreadActions - const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - calls++ - await Promise.resolve() - // Someone hits jump-to-recent while the first page is in flight. - if (calls === 1) { - clearVersion = 1 - } - p.onFullThread?.( - JSON.stringify({messages: tombstones(7151, 7052), pagination: {last: false, num: 100}}) - ) - return undefined as never - }) - loadBack(actions) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(1) - }) - - test('does not reload an initial load', async () => { - const rpc = mockWalkingBack(6152) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, trackingActions()) - await flushPromises() - expect(rpc).toHaveBeenCalledTimes(1) - }) -}) - -describe('a load releases the window gate it was issued under', () => { - const flushPromises = async () => { - for (let i = 0; i < 200; i++) { - await Promise.resolve() - } - } - - // clearVersion is the only thing that separates two loads of the same conversation: - // isThreadLoadCurrent is keyed on a generation that moves only when the conversation changes or - // the thread unmounts, so both loads call themselves current. - const gateActions = (clearVersion: () => number) => - ({ - applyThreadLoad: jest.fn(), - claimWindowGate: jest.fn(), - clearWindowGate: jest.fn(), - getSnapshot: () => - ({ - clearVersion: clearVersion(), - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: undefined, - pendingOutboxToOrdinal: new Map(), - }) as unknown as ConversationThreadState, - loadMoreMessages: jest.fn(), - markThreadAsRead: jest.fn(), - }) as unknown as ConversationThreadActions - - beforeEach(() => { - useCurrentUserState.getState().dispatch.setBootstrap({ - deviceID: 'device-id', - deviceName: 'testuser-mac', - uid: 'uid', - username: 'testuser', - }) - }) - - afterEach(() => { - jest.restoreAllMocks() - resetAllStores() - }) - - test('releases it when the load bails before the rpc is even made', async () => { - // The clear issues its reload synchronously, so if that reload is the one bailing there is - // nothing else coming to take the gate down and the thread stops receiving messages for good. - const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock') - const actions = gateActions(() => 3) - loadConversationThreadMessages( - conversationIDKey, - {isThreadLoadCurrent: () => false, reason: 'focused'}, - actions - ) - await flushPromises() - - expect(rpc).not.toHaveBeenCalled() - expect(actions.clearWindowGate).toHaveBeenCalledTimes(1) - }) - - test('releases it when the load ends without ever applying', async () => { - // A response that carries no thread: applyThreadLoad never runs, so nothing else would take the - // gate down. Left up it drops every notification for the life of the provider. - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async () => { - await Promise.resolve() - return undefined as never - }) - const actions = gateActions(() => 3) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(actions.clearWindowGate).toHaveBeenCalledTimes(1) - }) - - test('does not apply a response that arrives after a clear', async () => { - // The back page is in flight when the reader taps jump-to-recent: messagesClear empties the - // window and starts its own load. Applying this one anyway repopulates the window the clear - // dropped and lowers the gate the new load is relying on, and the two disjoint pages then - // merge - the stranded-row bug the gate exists to prevent. - let clearVersion = 3 - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - clearVersion = 4 - p.onFullThread?.( - JSON.stringify({ - messages: [ - { - placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(7152)}, - state: T.RPCChat.MessageUnboxedState.placeholder, - }, - ], - pagination: {last: false, num: 100}, - }) - ) - return undefined as never - }) - const actions = gateActions(() => clearVersion) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(actions.applyThreadLoad).not.toHaveBeenCalled() - expect(actions.clearWindowGate).not.toHaveBeenCalled() - }) - - test('does not apply a response while another load owns the gate', async () => { - // Two loads issued after the same clear: the reload the clear started owns the gate, and a - // ChatThreadsStale reload fired behind it answers first. clearVersion cannot tell them apart - - // it moved once, for the clear both of them started after. Applying this one would fill the - // cleared window with the newest page while the owner is still fetching a disjoint region, and - // the owner's page would then merge into it. - let claimed = -1 - const actions = { - applyThreadLoad: jest.fn(), - claimWindowGate: jest.fn((loadID: number) => { - claimed = loadID - }), - clearWindowGate: jest.fn(), - getSnapshot: () => - ({ - clearVersion: 3, - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: undefined, - pendingOutboxToOrdinal: new Map(), - // Someone else got here first. - windowCleared: true, - windowGateOwner: claimed + 1, - }) as unknown as ConversationThreadState, - loadMoreMessages: jest.fn(), - markThreadAsRead: jest.fn(), - } as unknown as ConversationThreadActions - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onFullThread?.( - JSON.stringify({ - messages: [ - { - placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(7152)}, - state: T.RPCChat.MessageUnboxedState.placeholder, - }, - ], - pagination: {last: false, num: 100}, - }) - ) - return undefined as never - }) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(actions.applyThreadLoad).not.toHaveBeenCalled() - }) - - test('applies the response of the load that owns the gate', async () => { - // The other half: the gate is up and this is the reload that claimed it, so it is the one - // allowed to refill the window. - let claimed = -1 - const actions = { - applyThreadLoad: jest.fn(), - claimWindowGate: jest.fn((loadID: number) => { - claimed = loadID - }), - clearWindowGate: jest.fn(), - getSnapshot: () => - ({ - clearVersion: 3, - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: undefined, - pendingOutboxToOrdinal: new Map(), - windowCleared: true, - windowGateOwner: claimed, - }) as unknown as ConversationThreadState, - loadMoreMessages: jest.fn(), - markThreadAsRead: jest.fn(), - } as unknown as ConversationThreadActions - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onFullThread?.( - JSON.stringify({ - messages: [ - { - placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(7152)}, - state: T.RPCChat.MessageUnboxedState.placeholder, - }, - ], - pagination: {last: false, num: 100}, - }) - ) - return undefined as never - }) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(actions.applyThreadLoad).toHaveBeenCalled() - }) - - test('leaves a newer clear’s gate alone', async () => { - // The load is in flight when the user taps a search result: messagesClear bumps clearVersion and - // starts its own load. This one must not pull down the gate that one is relying on - the load - // generation does not move between two loads of the same conversation, so it cannot tell them - // apart on its own. - let clearVersion = 3 - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async () => { - await Promise.resolve() - clearVersion = 4 - return undefined as never - }) - const actions = gateActions(() => clearVersion) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(actions.clearWindowGate).not.toHaveBeenCalled() - }) -}) -describe('only a pass that can account for a whole window reconciles', () => { - const flushPromises = async () => { - for (let i = 0; i < 200; i++) { - await Promise.resolve() - } - } - - const page = (from: number, to: number) => - Array.from({length: from - to + 1}, (_, i) => ({ - placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(from - i)}, - state: T.RPCChat.MessageUnboxedState.placeholder, - })) - - const recordingActions = () => - ({ - applyThreadLoad: jest.fn(), - claimWindowGate: jest.fn(), - clearWindowGate: jest.fn(), - getSnapshot: () => - ({ - clearVersion: 0, - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: undefined, - pendingOutboxToOrdinal: new Map(), - }) as unknown as ConversationThreadState, - loadMoreMessages: jest.fn(), - markThreadAsRead: jest.fn(), - }) as unknown as ConversationThreadActions - - const mockPasses = (cached: string, full: string) => - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onCachedThread?.(cached) - p.onFullThread?.(full) - return undefined as never - }) - - // Whether the last pass of a load was the one that reconciles. What gets pruned is the store's - // business - addMessagesToThreadState fills the carried set itself - so these tests only check - // which passes are allowed to ask for it; the thread-context suite covers the pruning. - const prunedOnLastPass = (actions: ConversationThreadActions) => { - const calls = (actions.applyThreadLoad as unknown as jest.Mock).mock.calls - return (calls.at(-1)?.[0] as {reconcile?: ThreadLoadReconcile} | undefined)?.reconcile?.prune - } - - beforeEach(() => { - useCurrentUserState.getState().dispatch.setBootstrap({ - deviceID: 'device-id', - deviceName: 'testuser-mac', - uid: 'uid', - username: 'testuser', - }) - }) - - afterEach(() => { - jest.restoreAllMocks() - resetAllStores() - }) - - test('reconciles on a full pass that followed an empty cached one', async () => { - // First open after a db nuke: PullLocalOnly finds nothing, but its collector suppresses the miss - // and a cached pass is sent anyway, carrying no messages. INCREMENTAL against an empty local - // thread filters nothing out, so the full pass really is the whole window - and only a whole - // window may prune the stale ordinals a cache repair left behind. - const actions = recordingActions() - mockPasses( - JSON.stringify({messages: null, pagination: {last: false, num: 100}}), - JSON.stringify({messages: page(7153, 7152), pagination: {last: false, num: 100}}) - ) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(prunedOnLastPass(actions)).toBe(true) - }) - - test('does not reconcile when a cached pass arrived but another load owned the window', async () => { - // The gate-owner guard is the one guard that can turn a cached pass away and still let the full - // pass behind it through: the owner drops the gate in between. The service counts that cached - // pass as sent either way, so the full pass is INCREMENTAL - a handful of changed messages - and - // a span built from those alone covers every row between them with nothing recorded as present. - // That is not a stale-row cleanup, it is deleting the thread. - let claimed = -1 - let ownedByAnother = true - const actions = { - applyThreadLoad: jest.fn(), - claimWindowGate: jest.fn((loadID: number) => { - claimed = loadID - }), - clearWindowGate: jest.fn(), - getSnapshot: () => - ({ - clearVersion: 0, - liveUpdateVersion: 0, - loaded: true, - messageIDToOrdinal: new Map(), - messageMap: new Map(), - messageOrdinals: undefined, - pendingOutboxToOrdinal: new Map(), - windowCleared: ownedByAnother, - windowGateOwner: claimed + 1, - }) as unknown as ConversationThreadState, - loadMoreMessages: jest.fn(), - markThreadAsRead: jest.fn(), - } as unknown as ConversationThreadActions - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onCachedThread?.( - JSON.stringify({messages: page(7153, 7052), pagination: {last: false, num: 100}}) - ) - // The load that owned the gate settles here, so the full pass is no longer refused. - ownedByAnother = false - p.onFullThread?.( - JSON.stringify({messages: page(7153, 7150), pagination: {last: false, num: 100}}) - ) - return undefined as never - }) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(actions.applyThreadLoad).not.toHaveBeenCalled() - }) - - test('does not reconcile when the service never reported a cached pass', async () => { - // The service records the cached thread as sent before it marshals it, so a failure there - // leaves the full pass INCREMENTAL against a pass we were never shown. The cached callback - // firing - with a thread, or with the nil a cold cache sends - is the only sign we get that - // this did not happen. - const actions = recordingActions() - jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { - await Promise.resolve() - p.onFullThread?.( - JSON.stringify({messages: page(7153, 7150), pagination: {last: false, num: 100}}) - ) - return undefined as never - }) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(prunedOnLastPass(actions)).toBe(false) - }) - - test('reconciles on the full pass of a warm-cache load, against both passes', async () => { - // The warm-cache sequence: the cached pass carries the page and the full pass behind it is - // INCREMENTAL, only what changed. Neither is a window on its own - but INCREMENTAL walks the - // authoritative window and omits only what the cached pass already carried unchanged, so the - // two together are that window, and the range spans both. Judging the full pass alone would - // give up pruning on every conversation the cache is warm for, which is all of them after the - // first open. - const actions = recordingActions() - mockPasses( - JSON.stringify({messages: page(7153, 7052), pagination: {last: false, num: 100}}), - JSON.stringify({messages: page(7153, 7153), pagination: {last: false, num: 100}}) - ) - loadConversationThreadMessages(conversationIDKey, {reason: 'focused'}, actions) - await flushPromises() - - expect(prunedOnLastPass(actions)).toBe(true) - }) -}) diff --git a/shared/chat/conversation/thread-load.tsx b/shared/chat/conversation/thread-load.tsx index 1798882eb82f..561dcf805ab6 100644 --- a/shared/chat/conversation/thread-load.tsx +++ b/shared/chat/conversation/thread-load.tsx @@ -1,36 +1,16 @@ import * as Common from '@/constants/chat/common' -import * as Message from '@/constants/chat/message' import * as Meta from '@/constants/chat/meta' -import * as Strings from '@/constants/strings' import * as T from '@/constants/types' -import {navigateToInbox} from '@/constants/router' import logger from '@/logger' import {findLast} from '@/util/arrays' import {ignorePromise} from '@/constants/utils' import {RPCError} from '@/util/errors' -import {persistRoute} from '@/util/storeless-actions' import {uint8ArrayToString} from '@/util/uint8array' import {useCurrentUserState} from '@/stores/current-user' import {useConfigState} from '@/stores/config' -import {type ThreadLoadReconcile, getOrdinalForMessageID} from './thread-message-state' -import {getInboxConversationMeta, updateInboxConversationMeta} from '@/chat/inbox/metadata' -import {loadThreadNonblock, threadLoadReasonToRPCReason} from './thread-rpc' -import type { - ConversationThreadActions, - ConversationThreadState, - LoadMoreMessagesParams, - ScrollDirection, -} from './thread-context' - -// Identifies one load, so the window gate can tell two loads of the same conversation apart. -// Only ever compared for equality, never ordered. -let nextLoadID = 0 - -export const numMessagesOnInitialLoad = isMobile ? 20 : 100 -// How far the no-new-ordinals back-page chain will walk on its own before handing the thread back to -// the reader. See the reload block in loadConversationThreadMessages. -export const maxBackPageReloads = 10 -export const numMessagesOnScrollback = 100 +import {getOrdinalForMessageID} from './thread-message-state' +import {getInboxConversationMeta} from '@/chat/inbox/metadata' +import type {ConversationThreadState, ScrollDirection} from './thread-context' const ignoreErrors = [ T.RPCGen.StatusCode.scgenericapierror, @@ -160,319 +140,3 @@ export const scrollDirectionToPagination = ( } return pagination } - -export const loadConversationThreadMessages = ( - conversationIDKey: T.Chat.ConversationIDKey, - p: LoadMoreMessagesParams, - actions: ConversationThreadActions -) => { - if (!T.Chat.isValidConversationIDKey(conversationIDKey)) { - return - } - const { - scrollDirection = 'none', - numberOfMessagesToLoad = numMessagesOnInitialLoad, - retryBelowMessageID, - retryCount = 0, - } = p - const { - allowMarkAsRead = true, - reason, - forceContainsLatestCalc, - messageIDControl, - knownRemotes, - centeredMessageID, - isThreadLoadCurrent, - onThreadLoadStatus, - } = p - const isCurrentThreadLoad = () => isThreadLoadCurrent?.() ?? true - - const f = async () => { - const loadStartedSnapshot = actions.getSnapshot() - const clearVersionAtLoadStart = loadStartedSnapshot.clearVersion - // applyThreadLoad drops the window gate when a load refills the window, but a load can end - // without ever applying: offline, scchatnotinteam, a response carrying no thread, or a bail - // before the RPC is even made. Left alone the gate would keep dropping notifications for the - // life of the provider, which is a thread that silently stops receiving messages. - // - // Keyed on clearVersion, not on isThreadLoadCurrent: the load generation only moves when the - // conversation changes or the thread unmounts, so two loads of the same conversation both call - // themselves current. A load that started before the clear would otherwise pull down the gate - // belonging to the load that started after it, while that one is still in flight. - // - // clearVersion alone still cannot separate two loads issued after the same clear, so the gate - // is also owned: first claim wins, and only the owner may drop it. Claimed here, before the - // first await, rather than when a response arrives - both clear paths bypass the load throttle - // (see loadMoreMessages in thread-context) and call in synchronously, so the reload the clear - // issued is always the first to get here, and a load that ends without ever applying still has - // to be the one that releases. - const loadID = nextLoadID++ - actions.claimWindowGate(loadID) - const releaseWindowGate = () => { - if (actions.getSnapshot().clearVersion === clearVersionAtLoadStart) { - actions.clearWindowGate(loadID) - } - } - // Every bail from here on releases, including the two that used to sit above the claim: the - // clear issues its reload synchronously, so if that reload is the one bailing there is nothing - // else coming to take the gate down, and the thread stops receiving messages for good. - if (!isCurrentThreadLoad()) { - logger.info('loadMoreMessages: bail: stale mounted thread load') - releaseWindowGate() - return - } - - if (!conversationIDKey || !T.Chat.isValidConversationIDKey(conversationIDKey)) { - logger.info('loadMoreMessages: bail: no conversationIDKey') - releaseWindowGate() - return - } - - const currentMeta = getMeta(conversationIDKey) - if (currentMeta.membershipType === 'youAreReset' || currentMeta.rekeyers.size > 0) { - logger.info('loadMoreMessages: bail: we are reset') - releaseWindowGate() - return - } - const loadStartedLiveUpdateVersion = loadStartedSnapshot.liveUpdateVersion - const protectLoadedFocusRefresh = - loadStartedSnapshot.loaded && - scrollDirection === 'none' && - !centeredMessageID && - !messageIDControl && - (reason === 'focused' || reason === 'tab selected') - logger.info( - `loadMoreMessages: calling rpc convo: ${conversationIDKey} num: ${numberOfMessagesToLoad} reason: ${reason}` - ) - - const loadingKey = Strings.waitingKeyChatThreadLoad(conversationIDKey) - // What this load has put in the window, filled in by addMessagesToThreadState as each pass - // applies. Once the service has sent a cached thread it switches the full response to - // INCREMENTAL, which walks the authoritative window and sends only the messages that cached - // pass did not already carry unchanged (mergeLocalRemoteThread in go/chat/uithreadloader.go, - // where localSentThread is that exact pass). Neither pass is a whole window on its own, so the - // two are gathered here and the last one reconciles against the both of them. - const carried = new Set() - // A load is all or nothing. Once one of its passes is turned away, the rest of them are too: - // the service filters each pass against what it has already sent this load, so the ones that - // follow a refused pass are a subset of a window we never took, and both ways of using them - // are wrong. Merging one into whatever refilled the window in the meantime is the disjoint - // window this whole invariant exists to prevent; reconciling against one takes out every row - // between the few messages it happens to carry. - let refusedAPass = false - // Whether the service's cached goroutine reported at all - with a thread, or with the nil it - // sends when the local cache had nothing. It is the only evidence the client gets that the - // full pass was not filtered behind our back: the service records the cached thread as sent - // before it marshals it, so a marshal failure there leaves us with an INCREMENTAL full pass - // and no sign of the pass it was filtered against (LoadNonblock in - // go/chat/uithreadloader.go). No report, no reconciling. - let sawCachedReport = false - // The reload below is judged against the whole load, not one pass of it. A warm-cache load - // delivers the page on the cached pass and then an INCREMENTAL full pass carrying only what - // changed, so measuring the full pass alone says "added nothing" for a perfectly good page. - // Measuring from before either pass tells the two apart: a page of real messages moves this, - // a page of tombstones does not, wherever it arrived. - const floorAtLoadStart = loadStartedSnapshot.messageOrdinals?.[0] - let oldestSeenThisLoad = Number.MAX_SAFE_INTEGER as T.Chat.MessageID - const onGotThread = (thread: string, why: string) => { - if (!thread) { - return - } - if (refusedAPass) { - logger.info(`loadMoreMessages: pass ignored, an earlier one of this load was: ${why}`) - return - } - const refuse = (msg: string) => { - refusedAPass = true - logger.info(msg) - } - if (!isCurrentThreadLoad()) { - refuse(`loadMoreMessages: stale response ignored: ${why}`) - return - } - // A clear under us - jump to recent, a centered jump - dropped the window this load was - // paging against, and the reload that follows fetches a disjoint region. isCurrentThreadLoad - // does not catch it: the load generation only moves when the conversation changes or the - // thread unmounts, so a load that started before the clear still calls itself current. - // Applying it anyway would repopulate the cleared window and lower the gate belonging to the - // reload, which then merges its own page into the leftovers. - const snapshotAtResponse = actions.getSnapshot() - if (snapshotAtResponse.clearVersion !== clearVersionAtLoadStart) { - refuse(`loadMoreMessages: response ignored after clear: ${why}`) - return - } - // clearVersion cannot separate two loads issued after the same clear, and the second one is - // not hypothetical: a ChatThreadsStale or ChatInboxSynced reload fires with scrollDirection - // 'none' and fetches the newest page, not the region the clear asked for. If it answers - // first it would fill the cleared window with that disjoint page and drop the gate, and the - // reload the clear issued would then merge its own page into the leftovers - exactly the - // ordinal gap the gate exists to prevent. While the gate is up only its owner may refill the - // window; once the owner settles the gate is down and everyone applies normally again. - if ( - snapshotAtResponse.windowCleared && - snapshotAtResponse.windowGateOwner !== undefined && - snapshotAtResponse.windowGateOwner !== loadID - ) { - refuse(`loadMoreMessages: response ignored, another load owns the window: ${why}`) - return - } - if (protectLoadedFocusRefresh && snapshotAtResponse.liveUpdateVersion !== loadStartedLiveUpdateVersion) { - refuse( - `loadMoreMessages: stale response ignored after live update: ${why} reason=${reason} convID=${conversationIDKey}` - ) - return - } - - const {username, devicename} = getCurrentUser() - const {messages, pagination} = Message.parseUIMessagesJSON( - conversationIDKey, - thread, - username, - devicename, - () => getLastOrdinalFromSnapshot(actions.getSnapshot()) - ) - const moreToLoad = pagination ? !pagination.last : true - const canMarkReadForThreadWindow = - allowMarkAsRead && - !centeredMessageID && - !messageIDControl && - scrollDirection !== 'back' && - reason !== 'findNewestConversation' && - reason !== 'findNewestConversationFromLayout' - // Reconciling is only safe against a whole window, and a single pass is not one: the cached - // pass is whatever the local cache holds, gaps included, and the full pass behind it carries - // only what changed. The full pass is the last one, so it is the one that prunes - against - // everything both passes delivered. Waiting instead for a pass with no cached one before it - // would leave the stale-row cleanup running on cold caches only, which is where ghost rows - // are least likely to be: a reopened conversation is warm every time. - const reconcile: ThreadLoadReconcile | undefined = - scrollDirection === 'none' ? {carried, prune: why === 'full' && sawCachedReport} : undefined - for (const m of messages) { - if (m.id > 0 && m.id < oldestSeenThisLoad) { - oldestSeenThisLoad = m.id - } - } - actions.applyThreadLoad({ - centered: !!centeredMessageID, - disableActiveMarkRead: !allowMarkAsRead || !!centeredMessageID || !!messageIDControl, - enableActiveMarkRead: canMarkReadForThreadWindow, - forceContainsLatestCalc, - messages, - moreToLoad, - reconcile, - scrollDirection, - }) - const after = actions.getSnapshot() - // A back page can be composed entirely of messages the thread will never render: a message - // superseded by a DELETE arrives as a hidden placeholder, becomes `deleted`, and addMessages - // drops it. The ordinal list is then identical to what it was, so the list never fires - // onStartReached again and scrollback stops even though the pager says there is more. Ask for - // the next page ourselves. - // - // The tombstones still carry message IDs, and each page reaches further back than the last, - // so requiring strict progress terminates: message IDs are finite and only ever decrease - // here. Strict progress alone is a weak bound though - a channel whose history was largely - // expunged has tens of thousands of them, which is minutes of paging off one gesture - so the - // chain also stops after maxBackPageReloads. Stopping is safe: the reader is still pinned at - // the top with an unchanged list, and scrolling away and back fires onStartReached again, - // which starts a fresh chain from wherever this one left off. - const floorAfter = after.messageOrdinals?.[0] - const windowGrewDownward = - floorAfter !== undefined && (floorAtLoadStart === undefined || floorAfter < floorAtLoadStart) - if ( - scrollDirection === 'back' && - // The full pass is the last one of a load, so by here the whole load has been applied. - why === 'full' && - moreToLoad && - // The floor, not the count: a page can add real messages while its `deleted` entries - // remove more from the window, which nets negative on a count but is real progress. - !windowGrewDownward && - oldestSeenThisLoad < (retryBelowMessageID ?? Number.MAX_SAFE_INTEGER) && - retryCount < maxBackPageReloads - ) { - logger.info( - `loadMoreMessages: back page added no ordinals, reloading below ${oldestSeenThisLoad} (${ - retryCount + 1 - }/${maxBackPageReloads}): convID: ${conversationIDKey}` - ) - // Through the action, not loadConversationThreadMessages directly: the action carries the - // 500ms throttle and the unmount cancel(), and a long run of tombstones would otherwise - // issue these back to back with no pacing. The throttle only ever drops a call that a - // later load supersedes, and that load extends the window or retries in turn. - // - // The delay has a cost: the next page comes from a cursor the daemon holds, not one we - // send. pgmode is SERVER (see thread-rpc), so `next` resolves against convPageStatus in the - // service, and any first-page request resets it (applyPagerModeOutgoing in - // go/chat/uithreadloader.go) - which every scrollDirection 'none' load is, stale and focus - // reloads included. One landing inside the throttle window makes this retry fetch near the - // top of the thread instead of the next page back. It fails closed rather than looping: - // oldestSeenThisLoad is then no lower than retryBelowMessageID, so the chain stops and the - // reader is left where another scroll gesture starts a fresh one. - // - // Sizing, for the same reason the chain is bounded at all: a full run is 11 sequential - // 100-message RPCs off one gesture, several seconds of paging with nothing visible moving. - actions.loadMoreMessages({ - ...p, - retryBelowMessageID: oldestSeenThisLoad, - retryCount: retryCount + 1, - }) - } - - if (canMarkReadForThreadWindow) { - actions.markThreadAsRead() - } - } - - const pagination = messageIDControl - ? null - : scrollDirectionToPagination(scrollDirection, numberOfMessagesToLoad) - try { - const results = await loadThreadNonblock({ - conversationIDKey, - knownRemotes, - messageIDControl, - onCachedThread: thread => { - sawCachedReport = true - onGotThread(thread, 'cached') - }, - onFullThread: thread => onGotThread(thread, 'full'), - onThreadStatus: status => { - logger.info( - `loadMoreMessages: thread status received: convID: ${conversationIDKey} typ: ${status.typ}` - ) - if (isCurrentThreadLoad()) { - onThreadLoadStatus?.(conversationIDKey, status.typ) - } - }, - pagination, - reason: threadLoadReasonToRPCReason(reason), - waitingKey: loadingKey, - }) - if (!isCurrentThreadLoad()) { - return - } - updateInboxConversationMeta(conversationIDKey, {offline: results.offline}) - } catch (error) { - if (!isCurrentThreadLoad()) { - return - } - if (error instanceof RPCError) { - logger.warn(`loadMoreMessages: error: ${error.desc}`) - if (error.code === T.RPCGen.StatusCode.scchatnotinteam) { - // We're no longer in this conv's team. Clear the persisted last-route - // (ui.routeState2) so app startup doesn't keep restoring and reloading - // this conv, which would re-trigger this error on every launch. - persistRoute(true, true, () => useConfigState.getState().startup.loaded) - navigateToInbox(true, 'maybeKickedFromTeam') - } - if (error.code !== T.RPCGen.StatusCode.scteamreaderror) { - throw error - } - } - } finally { - releaseWindowGate() - } - } - - ignorePromise(f()) -} diff --git a/shared/chat/conversation/thread-window.test.tsx b/shared/chat/conversation/thread-window.test.tsx new file mode 100644 index 000000000000..e0c455fd0ac6 --- /dev/null +++ b/shared/chat/conversation/thread-window.test.tsx @@ -0,0 +1,1679 @@ +/** @jest-environment jsdom */ +/// +import * as Common from '@/constants/chat/common' +import * as Message from '@/constants/chat/message' +import * as Meta from '@/constants/chat/meta' +import * as T from '@/constants/types' +import * as ThreadRpc from './thread-rpc' +import HiddenString from '@/util/hidden-string' +import {act, cleanup, renderHook} from '@testing-library/react' +import type * as React from 'react' +import {metasReceived} from '@/chat/inbox/metadata' +import {notifyEngineActionListeners} from '@/engine/action-listener' +import {resetAllStores} from '@/util/zustand' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useShellState} from '@/stores/shell' +import { + ConversationThreadProvider, + type ConversationThreadActions, + type ConversationThreadState, + useConversationThreadActions, + useConversationThreadMarkThreadAsRead, + useConversationThreadMessage, + useConversationThreadSelector, + useConversationThreadStore, +} from './thread-context' +import { + ConversationThreadWindowProvider, + maxBackPageReloads, + numMessagesOnScrollback, + runThreadWindowLoad, + useRequestWindow, + useThreadLoadStatus, + useThreadWindow, +} from './thread-window' + +const convID = T.Chat.conversationIDToKey(new Uint8Array([1, 2, 3, 4])) +const otherConvID = T.Chat.conversationIDToKey(new Uint8Array([5, 6, 7, 8])) + +const flushPromises = async () => { + for (let i = 0; i < 200; i++) { + await Promise.resolve() + } +} + +const textAt = (n: number) => + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id: T.Chat.numberToMessageID(n), + ordinal: T.Chat.numberToOrdinal(n), + text: new HiddenString(`message ${n}`), + timestamp: 100, + }) + +const makeTextMessage = () => + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id: T.Chat.numberToMessageID(301), + ordinal: T.Chat.numberToOrdinal(301), + outboxID: T.Chat.stringToOutboxID('outbox-1'), + text: new HiddenString('stale message'), + timestamp: 100, + }) + +const makeValidTextUIMessage = ( + serverMsgID: T.Chat.MessageID, + text: string, + outboxID = '' +): T.RPCChat.UIMessage => ({ + state: T.RPCChat.MessageUnboxedState.valid, + valid: { + atMentions: null, + bodySummary: text, + botUsername: '', + channelMention: T.RPCChat.ChannelMention.none, + channelNameMentions: null, + ctime: 200, + decoratedTextBody: null, + etime: 0, + explodedBy: null, + hasPairwiseMacs: false, + isCollapsed: false, + isDeleteable: true, + isEditable: true, + isEphemeral: false, + isEphemeralExpired: false, + messageBody: { + messageType: T.RPCChat.MessageType.text, + text: { + body: text, + payments: null, + replyTo: null, + replyToUID: null, + teamMentions: null, + userMentions: null, + }, + }, + messageID: T.Chat.messageIDToNumber(serverMsgID), + outboxID, + paymentInfos: null, + pinnedMessageID: null, + reactions: {}, + replyTo: null, + requestInfo: null, + senderDeviceID: new Uint8Array([1]), + senderDeviceName: 'bob-device', + senderDeviceRevokedAt: null, + senderDeviceType: 'desktop', + senderUID: new Uint8Array([2]), + senderUsername: 'bob', + superseded: false, + unfurls: null, + }, +}) + +const threadJSON = (msgIDs: ReadonlyArray, last = true) => + JSON.stringify({ + messages: msgIDs.map(id => makeValidTextUIMessage(id, `m${id}`)), + pagination: {last, next: '', num: 100, previous: ''}, + }) + +type WindowProviderProps = { + allowMarkReadOnLoad?: boolean + skipThreadLoadOnSelection?: boolean +} + +const makeWrapper = (p: WindowProviderProps = {}) => { + const {allowMarkReadOnLoad = true, skipThreadLoadOnSelection = true} = p + return function Wrapper({children}: {children: React.ReactNode}) { + return ( + + + {children} + + + ) + } +} + +// The whole public surface of the module, in one hook, so a test reads as "ask for a window, then +// look at the window". +const useHarness = () => ({ + actions: useConversationThreadActions(), + markThreadAsRead: useConversationThreadMarkThreadAsRead(), + requestWindow: useRequestWindow(), + status: useThreadLoadStatus(), + window: useThreadWindow(), +}) + +const renderWindow = (p: WindowProviderProps = {}) => renderHook(useHarness, {wrapper: makeWrapper(p)}) + +beforeEach(() => { + jest.spyOn(T.RPCChat, 'localRequestInboxUnboxRpcPromise').mockResolvedValue(undefined) + useCurrentUserState.getState().dispatch.setBootstrap({ + deviceID: 'device-id', + deviceName: 'test-device', + uid: 'uid', + username: 'alice', + }) + metasReceived( + [{...Meta.makeConversationMeta(), conversationIDKey: convID, readMsgID: T.Chat.numberToMessageID(0)}], + undefined, + {force: true} + ) +}) + +afterEach(() => { + cleanup() + jest.restoreAllMocks() + resetAllStores() +}) + +describe('requestWindow turns a place in the thread into a load', () => { + test('a centered anchor drops the window it replaces and pivots the rpc on the message', async () => { + const loadThread = jest + .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') + .mockResolvedValue({offline: false}) + const {result} = renderHook( + () => ({...useHarness(), staleMessage: useConversationThreadMessage(T.Chat.numberToOrdinal(301))}), + {wrapper: makeWrapper()} + ) + + act(() => { + result.current.actions.addMessages([makeTextMessage()]) + }) + expect(result.current.staleMessage?.id).toBe(T.Chat.numberToMessageID(301)) + + act(() => { + result.current.requestWindow({ + anchor: {centeredOn: T.Chat.numberToMessageID(999)}, + reason: 'centered', + }) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.staleMessage).toBeUndefined() + expect(result.current.window.ordinals).toEqual([]) + expect(loadThread).toHaveBeenCalledWith( + expect.objectContaining({ + params: expect.objectContaining({ + query: expect.objectContaining({ + messageIDControl: expect.objectContaining({ + mode: T.RPCChat.MessageIDControlMode.centered, + pivot: T.Chat.numberToMessageID(999), + }), + }), + }), + }) + ) + }) + + test('jump to recent reloads the newest page, reports status and marks the thread read', async () => { + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + const markAsRead = jest + .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') + .mockResolvedValue({offline: false}) + const msgID = T.Chat.numberToMessageID(202) + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadStatus']?.({ + status: {typ: T.RPCChat.UIChatThreadStatusTyp.server}, + }) + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({thread: threadJSON([msgID])}) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'jump to recent'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.status).toBe(T.RPCChat.UIChatThreadStatusTyp.server) + expect(markAsRead).toHaveBeenCalledWith({ + conversationID: T.Chat.keyToConversationID(convID), + forceUnread: false, + msgID, + }) + }) + + test('jump to recent drops the old window instead of merging a disjoint one into it', async () => { + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(9001)]), + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + // The reader is deep in old history. + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [textAt(101), textAt(102)], + moreToLoad: true, + scrollDirection: 'none', + }) + }) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'jump to recent'}) + }) + await act(async () => { + await flushPromises() + }) + + // Only the newest window survives. If the old one were merged in, ordinals would read + // [101, 102, 9001] with an 8899-wide hole. + expect(result.current.window.ordinals).toEqual([T.Chat.numberToOrdinal(9001)]) + }) + + test('an older anchor is refused once the window reaches the oldest message', async () => { + const rpc = jest + .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') + .mockResolvedValue({offline: false}) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [textAt(101)], + moreToLoad: false, + scrollDirection: 'back', + }) + }) + act(() => { + result.current.requestWindow({anchor: 'older', reason: 'scroll back'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(rpc).not.toHaveBeenCalled() + }) + + test('a newer anchor is refused once the window reaches the latest message', async () => { + const rpc = jest + .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') + .mockResolvedValue({offline: false}) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [textAt(101)], + moreToLoad: false, + scrollDirection: 'none', + }) + }) + expect(result.current.window.moreToLoadForward).toBe(false) + act(() => { + result.current.requestWindow({anchor: 'newer', reason: 'scroll forward'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(rpc).not.toHaveBeenCalled() + }) + + test('scrollback loads older messages without marking the thread read', async () => { + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(201)], false), + }) + await Promise.resolve() + return {offline: false} + }) + const markAsRead = jest + .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') + .mockResolvedValue({offline: false}) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [makeTextMessage()], + moreToLoad: true, + scrollDirection: 'back', + }) + }) + act(() => { + result.current.requestWindow({anchor: 'older', reason: 'scroll back'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(markAsRead).not.toHaveBeenCalled() + }) + + test('a provider that disallows mark read on load does not arm active or explicit mark read', async () => { + useConfigState.setState({loggedIn: true}) + useShellState.getState().dispatch.setActive(false) + jest + .spyOn(Common, 'isUserActivelyLookingAtThisThread') + .mockImplementation(() => useShellState.getState().active) + const markAsRead = jest + .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') + .mockResolvedValue({offline: false}) + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(203)]), + }) + await Promise.resolve() + return {offline: false} + }) + // The mount-time selection load is the one allowMarkReadOnLoad governs. + const {result} = renderWindow({allowMarkReadOnLoad: false, skipThreadLoadOnSelection: false}) + + await act(async () => { + await flushPromises() + }) + expect(markAsRead).not.toHaveBeenCalled() + + act(() => { + useShellState.getState().dispatch.setActive(true) + }) + await act(async () => { + await flushPromises() + }) + expect(markAsRead).not.toHaveBeenCalled() + + act(() => { + result.current.markThreadAsRead() + }) + await act(async () => { + await flushPromises() + }) + expect(markAsRead).not.toHaveBeenCalled() + }) + + test('the mount-time selection load can be skipped for a thread about to be centered', async () => { + const rpc = jest + .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') + .mockResolvedValue({offline: false}) + renderWindow({skipThreadLoadOnSelection: true}) + await act(async () => { + await flushPromises() + }) + expect(rpc).not.toHaveBeenCalled() + + cleanup() + renderWindow({skipThreadLoadOnSelection: false}) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(1) + }) +}) + +// The full jump -> scroll-to-bottom -> stale-reload chain behind normal/container.tsx's +// allowMarkReadOnLoad. Jumping to a highlighted message mounts the thread with +// skipThreadLoadOnSelection (the centered load replaces the select-on-mount load) and blocks +// mark-read. The block is NOT permanent: applyThreadLoad releases it as soon as the user scrolls +// to the latest message ('forward' with no moreToLoad). The stale reload that follows - +// ChatThreadsStale fires on every mobile background -> foreground - must then be free to mark the +// thread read. The stale reload reads allowMarkReadOnLoad through useEffectEvent, i.e. the latest +// render's value, so a caller that derived it from the one-shot highlight and froze it at `false` +// would leave the conversation badged unread for as long as the thread stayed mounted. +const staleThreadUpdate = { + payload: { + params: { + uid: '', + updates: [ + {convID: T.Chat.keyToConversationID(convID), updateType: T.RPCChat.StaleUpdateType.newactivity}, + ], + }, + }, + type: 'chat.1.NotifyChat.ChatThreadsStale', +} as never + +describe('a stale thread reloads the newest page', () => { + const renderJumpedThenScrolledToBottom = (allowMarkReadOnLoad: boolean) => { + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + const markAsRead = jest + .spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise') + .mockResolvedValue({offline: false}) + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(203)]), + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow({allowMarkReadOnLoad, skipThreadLoadOnSelection: true}) + // jumping to a highlighted message blocks mark-read + act(() => { + result.current.actions.setMarkReadBlocked(true) + }) + // ...then the user scrolls all the way forward to the latest message, releasing the block + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [makeTextMessage()], + moreToLoad: false, + scrollDirection: 'forward', + }) + }) + return markAsRead + } + + test('a stale reload after a jump and a scroll to the bottom marks the thread read', async () => { + const markAsRead = renderJumpedThenScrolledToBottom(true) + + act(() => { + notifyEngineActionListeners(staleThreadUpdate) + }) + await act(async () => { + await flushPromises() + }) + + expect(markAsRead).toHaveBeenCalledTimes(1) + }) + + // The counterfactual: exactly what deriving allowMarkReadOnLoad from the one-shot highlight did. + test('a stale reload that disallows mark read leaves the thread unread even once the block is gone', async () => { + const markAsRead = renderJumpedThenScrolledToBottom(false) + + act(() => { + notifyEngineActionListeners(staleThreadUpdate) + }) + await act(async () => { + await flushPromises() + }) + + expect(markAsRead).not.toHaveBeenCalled() + }) + + test('a stale reload reports its thread status through the provider', async () => { + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadStatus']?.({ + status: {typ: T.RPCChat.UIChatThreadStatusTyp.server}, + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + act(() => { + notifyEngineActionListeners(staleThreadUpdate) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.status).toBe(T.RPCChat.UIChatThreadStatusTyp.server) + }) + + test('a stale thread notification for another conversation is ignored', async () => { + const rpc = jest + .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') + .mockResolvedValue({offline: false}) + renderWindow() + + act(() => { + notifyEngineActionListeners({ + payload: { + params: { + uid: '', + updates: [ + { + convID: T.Chat.keyToConversationID(otherConvID), + updateType: T.RPCChat.StaleUpdateType.newactivity, + }, + ], + }, + }, + type: 'chat.1.NotifyChat.ChatThreadsStale', + } as never) + }) + await act(async () => { + await flushPromises() + }) + + expect(rpc).not.toHaveBeenCalled() + }) + + test('an incremental inbox sync carrying this conversation reloads it too', async () => { + const rpc = jest + .spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener') + .mockResolvedValue({offline: false}) + renderWindow() + + act(() => { + notifyEngineActionListeners({ + payload: { + params: { + syncRes: { + incremental: { + items: [{conv: {convID: T.Chat.conversationIDKeyToString(convID)}}], + }, + syncType: T.RPCChat.SyncInboxResType.incremental, + }, + }, + }, + type: 'chat.1.NotifyChat.ChatInboxSynced', + } as never) + }) + await act(async () => { + await flushPromises() + }) + + expect(rpc).toHaveBeenCalledTimes(1) + }) +}) + +describe('a response may only become the window it was fetched against', () => { + test('a refreshed window does not overwrite reaction updates streamed into it', async () => { + const targetMsgID = T.Chat.numberToMessageID(301) + const targetOrdinal = T.Chat.numberToOrdinal(301) + let incomingCallMap: + | Parameters[0]['incomingCallMap'] + | undefined + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + incomingCallMap = p.incomingCallMap + await Promise.resolve() + return {offline: false} + }) + const {result} = renderHook( + () => ({...useHarness(), message: useConversationThreadMessage(targetOrdinal)}), + {wrapper: makeWrapper()} + ) + + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: true, + messages: [makeTextMessage()], + moreToLoad: false, + scrollDirection: 'none', + }) + }) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'tab selected'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(incomingCallMap).toBeDefined() + + act(() => { + notifyEngineActionListeners({ + payload: { + params: { + activity: { + activityType: T.RPCChat.ChatActivityType.reactionUpdate, + reactionUpdate: { + convID: T.Chat.keyToConversationID(convID), + reactionUpdates: [ + { + reactions: { + reactions: { + ':+1:': { + decorated: ':+1:', + users: { + alice: { + ctime: 300, + reactionMsgID: T.Chat.messageIDToNumber(T.Chat.numberToMessageID(99)), + }, + }, + }, + }, + }, + targetMsgID: T.Chat.messageIDToNumber(targetMsgID), + }, + ], + userReacjis: {skinTone: T.RPCGen.ReacjiSkinTone.none, topReacjis: null}, + }, + }, + }, + }, + type: 'chat.1.NotifyChat.NewChatActivity', + } as never) + }) + + expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) + + act(() => { + incomingCallMap?.['chat.1.chatUi.chatThreadFull']?.({ + thread: JSON.stringify({ + messages: [makeValidTextUIMessage(targetMsgID, 'stale server copy')], + pagination: {last: true, next: '', num: 20, previous: ''}, + }), + }) + }) + + expect(result.current.message?.reactions?.get(':+1:')?.users.map(u => u.username)).toEqual(['alice']) + }) + + test('a stale reload does not merge the newest page into a centered window', async () => { + // The reader taps a search result and sits on the window around it, with more to load forward. + // A ChatThreadsStale reload fetches the newest page, which is nowhere near that window: merging + // the two leaves ordinals with a hole through the middle and then calls the result the latest + // message, which is the gap this invariant is about. + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(9900), T.Chat.numberToMessageID(9901)], false), + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: true, + enableActiveMarkRead: false, + messages: [textAt(7000), textAt(7001)], + moreToLoad: true, + scrollDirection: 'none', + }) + }) + expect(result.current.window.moreToLoadForward).toBe(true) + + act(() => { + notifyEngineActionListeners(staleThreadUpdate) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.window.ordinals).toEqual([7000, 7001]) + // ...and the window still knows it has not reached the latest message. + expect(result.current.window.moreToLoadForward).toBe(true) + }) + + test('a newest page that reaches the window is still merged', async () => { + // The other side of the rule. A reader near the bottom gets a page that overlaps what they + // hold, so there is no hole to open and the refresh must land. + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(9901), T.Chat.numberToMessageID(9902)], false), + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: true, + enableActiveMarkRead: false, + messages: [textAt(9900), textAt(9901)], + moreToLoad: true, + scrollDirection: 'none', + }) + }) + + act(() => { + notifyEngineActionListeners(staleThreadUpdate) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.window.ordinals).toEqual([9900, 9901, 9902]) + }) + + test('a response arriving after the provider unmounts is not applied', async () => { + let incomingCallMap: + | Parameters[0]['incomingCallMap'] + | undefined + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + incomingCallMap = p.incomingCallMap + await Promise.resolve() + return {offline: false} + }) + let snapshot: (() => ConversationThreadState) | undefined + const {result, unmount} = renderHook( + () => { + const store = useConversationThreadStore() + snapshot = () => store.getState() + return useHarness() + }, + {wrapper: makeWrapper()} + ) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'focused'}) + }) + await act(async () => { + await flushPromises() + }) + expect(incomingCallMap).toBeDefined() + + unmount() + + act(() => { + incomingCallMap?.['chat.1.chatUi.chatThreadFull']?.({ + thread: threadJSON([T.Chat.numberToMessageID(9001)]), + }) + }) + + expect(snapshot?.().messageOrdinals).toBeUndefined() + }) +}) + +// The gate rules are the sharpest part of the module and the hardest to reach through a rendered +// list, so they are driven straight at the arbitration entry point - with the real store and the +// real actions behind it, and a real gate record rather than hand-set flags. +describe('the window gate', () => { + type LoadContext = Omit[0], 'load' | 'reload'> + type WindowLoad = Parameters[0]['load'] + + const newestLoad = (over: Partial = {}): WindowLoad => ({ + allowMarkAsRead: true, + numberOfMessagesToLoad: 100, + reason: 'focused', + retryCount: 0, + scrollDirection: 'none', + ...over, + }) + + const renderContext = () => { + let mounted = true + const {result} = renderHook( + () => ({ + actions: useConversationThreadActions(), + ordinals: useConversationThreadSelector(s => s.messageOrdinals), + store: useConversationThreadStore(), + }), + { + wrapper: ({children}: {children: React.ReactNode}) => ( + {children} + ), + } + ) + const context: LoadContext = { + actions: result.current.actions, + conversationIDKey: convID, + gate: {nextLoadID: 0, refillOwner: undefined}, + isMounted: () => mounted, + onThreadLoadStatus: () => {}, + store: result.current.store, + } + return { + context, + result, + unmountProvider: () => { + mounted = false + }, + } + } + + const drive = (context: LoadContext, load: WindowLoad) => { + runThreadWindowLoad({ + ...context, + load, + reload: (next: WindowLoad) => { + drive(context, next) + }, + }) + } + + test('only the load that claimed the gate may drop it', async () => { + // The generation cannot separate two loads of the same conversation: it moves once, for the + // clear both of them started after. The reader taps a search result, messagesClear issues the + // centered reload, and a ChatThreadsStale notification then fires a second load. If that one + // settles first - no thread, an error - it would take the gate down while the reload is still + // in flight, and a push landing in what is left of the gap strands exactly as it did before + // the gate existed. + let release: (() => void) | undefined + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation( + async () => + new Promise(resolve => { + release = () => resolve(undefined as never) + }) + ) + const {context, result} = renderContext() + act(() => { + result.current.actions.messagesClear() + }) + + // The reload the clear issued claims the gate... + drive(context, newestLoad({reason: 'centered'})) + await act(async () => { + await flushPromises() + }) + const owner = release + release = undefined + + // ...and the stale-thread load behind it loses the race. + drive(context, newestLoad({reason: 'got stale'})) + await act(async () => { + await flushPromises() + }) + await act(async () => { + release?.() + await flushPromises() + }) + expect(context.store.getState().windowCleared).toBe(true) + + await act(async () => { + owner?.() + await flushPromises() + }) + expect(context.store.getState().windowCleared).toBe(false) + }) + + test('releases the gate when the load bails before the rpc is even made', async () => { + // The clear issues its reload synchronously, so if that reload is the one bailing there is + // nothing else coming to take the gate down and the thread stops receiving messages for good. + const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock') + const {context, result, unmountProvider} = renderContext() + act(() => { + result.current.actions.messagesClear() + }) + unmountProvider() + + drive(context, newestLoad()) + await act(async () => { + await flushPromises() + }) + + expect(rpc).not.toHaveBeenCalled() + expect(context.store.getState().windowCleared).toBe(false) + }) + + test('releases the gate when the load ends without ever applying', async () => { + // A response that carries no thread: nothing else would take the gate down, and left up it + // drops every notification for the life of the provider. + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async () => { + await Promise.resolve() + return undefined as never + }) + const {context, result} = renderContext() + act(() => { + result.current.actions.messagesClear() + }) + + drive(context, newestLoad()) + await act(async () => { + await flushPromises() + }) + + expect(context.store.getState().windowCleared).toBe(false) + }) + + test('does not apply a response that arrives after a clear, and leaves the new gate alone', async () => { + // The back page is in flight when the reader taps jump-to-recent: messagesClear empties the + // window and starts its own load. Applying this one anyway repopulates the window the clear + // dropped and lowers the gate the new load is relying on, and the two disjoint pages then + // merge - the stranded-row bug the gate exists to prevent. + const {context, result} = renderContext() + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + act(() => { + result.current.actions.messagesClear() + }) + p.onFullThread?.(threadJSON([T.Chat.numberToMessageID(7152)], false)) + return undefined as never + }) + + drive(context, newestLoad()) + await act(async () => { + await flushPromises() + }) + + expect(result.current.ordinals).toBeUndefined() + expect(context.store.getState().windowCleared).toBe(true) + }) + + test('the load that owns the gate refills the window and drops it', async () => { + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onFullThread?.(threadJSON([T.Chat.numberToMessageID(7152)], false)) + return undefined as never + }) + const {context, result} = renderContext() + act(() => { + result.current.actions.messagesClear() + }) + + drive(context, newestLoad({reason: 'centered'})) + await act(async () => { + await flushPromises() + }) + + expect(result.current.ordinals).toEqual([T.Chat.numberToOrdinal(7152)]) + expect(context.store.getState().windowCleared).toBe(false) + }) + + test('an empty pass during a jump-to-recent gap leaves the gate up', async () => { + // A cold cache sends a cached pass carrying no messages ahead of the full response. Dropping + // the gate on it reopens the gap: a notification landing before the real page becomes the sole + // ordinal, and the page that follows is disjoint from it. + let sendFull: (() => void) | undefined + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onCachedThread?.(JSON.stringify({messages: null, pagination: {last: false, num: 100}})) + return new Promise(resolve => { + sendFull = () => { + p.onFullThread?.(threadJSON([T.Chat.numberToMessageID(9001)], false)) + resolve(undefined as never) + } + }) + }) + const {context, result} = renderContext() + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [textAt(7152), textAt(7153)], + moreToLoad: true, + scrollDirection: 'none', + }) + }) + act(() => { + result.current.actions.messagesClear() + }) + + drive(context, newestLoad({reason: 'jump to recent'})) + await act(async () => { + await flushPromises() + }) + expect(context.store.getState().windowCleared).toBe(true) + + // A push landing in the gap cannot install itself as the whole window. + act(() => { + result.current.actions.addMessages([textAt(7155)], {liveUpdate: true}) + }) + await act(async () => { + sendFull?.() + await flushPromises() + }) + + expect(result.current.ordinals).toEqual([T.Chat.numberToOrdinal(9001)]) + }) +}) + +describe('a back page that adds no ordinals reloads itself', () => { + type LoadContext = Omit[0], 'load' | 'reload'> + type WindowLoad = Parameters[0]['load'] + + // A real store seeded with a window, so a page's productivity is judged the way the thread judges + // it: a `deleted` message adds no ordinal, a renderable one does. + const contextFor = (store: LoadContext['store'], actions: ConversationThreadActions) => { + const context: LoadContext = { + actions, + conversationIDKey: convID, + gate: {nextLoadID: 0, refillOwner: undefined}, + isMounted: () => true, + onThreadLoadStatus: () => {}, + store, + } + return context + } + + const drive = (context: LoadContext, load: WindowLoad) => { + runThreadWindowLoad({ + ...context, + load, + reload: (next: WindowLoad) => { + drive(context, next) + }, + }) + } + + // Hidden placeholders are what a DELETE-superseded message arrives as, and what becomes `deleted` + // on this side. They carry real message IDs, which is what bounds the reload. + const tombstones = (from: number, to: number) => + Array.from({length: from - to + 1}, (_, i) => ({ + placeholder: {hidden: true, messageID: T.Chat.numberToMessageID(from - i)}, + state: T.RPCChat.MessageUnboxedState.placeholder, + })) + + // hidden: false parses to a `placeholder`, which the thread does render and keep an ordinal for. + const visible = (from: number, to: number) => + Array.from({length: from - to + 1}, (_, i) => ({ + placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(from - i)}, + state: T.RPCChat.MessageUnboxedState.placeholder, + })) + + // Each call walks one page further back, exactly as the service does, until it runs out. + const mockWalkingBack = (oldestOverall: number) => { + let next = 7151 + return jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + const from = next + const to = Math.max(oldestOverall, from - numMessagesOnScrollback + 1) + next = to - 1 + await Promise.resolve() + p.onFullThread?.( + JSON.stringify({messages: tombstones(from, to), pagination: {last: to <= oldestOverall, num: 100}}) + ) + return undefined as never + }) + } + + const seededContext = () => { + const {result} = renderHook( + () => ({actions: useConversationThreadActions(), store: useConversationThreadStore()}), + { + wrapper: ({children}: {children: React.ReactNode}) => ( + {children} + ), + } + ) + const {actions} = result.current + act(() => { + actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [textAt(7152), textAt(7153)], + moreToLoad: true, + scrollDirection: 'back', + }) + }) + return contextFor(result.current.store, result.current.actions) + } + + const loadBack = (context: LoadContext) => + drive(context, { + allowMarkAsRead: true, + numberOfMessagesToLoad: numMessagesOnScrollback, + reason: 'scroll back', + retryCount: 0, + scrollDirection: 'back', + }) + + test('keeps paging through a run of tombstones until the pager says it is done', async () => { + // 7151 down to 6952 is two pages of 100, so one reload after the first call. + const rpc = mockWalkingBack(6952) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(2) + }) + + test('walks a run of tombstones that ends before the cap', async () => { + const oldest = 6752 + const rpc = mockWalkingBack(oldest) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(Math.ceil((7151 - oldest + 1) / numMessagesOnScrollback)) + }) + + test('stops at the reload cap rather than walking an expunged history', async () => { + // A channel whose history was largely expunged has far more tombstones than the chain should + // walk off one gesture. It stops at the cap and hands the thread back; scrolling away and back + // fires onStartReached again and starts a fresh chain from where this one stopped. + const rpc = mockWalkingBack(1) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(maxBackPageReloads + 1) + }) + + test('stops if a page fails to reach further back', async () => { + // A service that keeps handing back the same window must not spin us forever. Progress in + // message ID is the only thing permitting another attempt. + const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onFullThread?.( + JSON.stringify({messages: tombstones(7151, 7052), pagination: {last: false, num: 100}}) + ) + return undefined as never + }) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(2) + }) + + test('does not reload when the page actually added ordinals', async () => { + // Renderable messages, so the store grows and the list will ask for the next page itself. + const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onFullThread?.(JSON.stringify({messages: visible(7151, 7052), pagination: {last: false, num: 100}})) + return undefined as never + }) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(1) + }) + + test('reloads when a warm cache delivers the tombstones', async () => { + // The reported bug's own shape: the conversation is already in local storage, so PullLocalOnly + // wins and the cached pass carries the page - which is entirely tombstones. Judging only the + // full pass, or refusing to judge at all once a cached pass arrived, leaves this inert. + let next = 7151 + const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + const from = next + const to = Math.max(6952, from - numMessagesOnScrollback + 1) + next = to - 1 + await Promise.resolve() + p.onCachedThread?.( + JSON.stringify({messages: tombstones(from, to), pagination: {last: to <= 6952, num: 100}}) + ) + // The full pass is INCREMENTAL once a cached thread has been sent. + p.onFullThread?.( + JSON.stringify({messages: tombstones(to, to), pagination: {last: to <= 6952, num: 100}}) + ) + return undefined as never + }) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(2) + }) + + test('does not reload after a cached pass already delivered the page', async () => { + // The normal warm-cache sequence: PullLocalOnly wins, the cached pass carries the whole page, + // and the full pass that follows is INCREMENTAL - only the messages that changed, every one of + // them already in the window. On ordinal count alone that is indistinguishable from a page of + // tombstones, and reloading on it walks the client back through the entire conversation. + const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onCachedThread?.(JSON.stringify({messages: visible(7151, 7052), pagination: {last: false, num: 100}})) + p.onFullThread?.(JSON.stringify({messages: visible(7052, 7052), pagination: {last: false, num: 100}})) + return undefined as never + }) + loadBack(seededContext()) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(1) + }) + + test('stops when the window is cleared under it', async () => { + // jump to recent and a centered jump both clear then reload. A chain still walking backwards + // would prepend pages into a window the reader has just left, producing the disjoint ordinals + // this whole branch exists to prevent. + let calls = 0 + const {result} = renderHook( + () => ({actions: useConversationThreadActions(), store: useConversationThreadStore()}), + { + wrapper: ({children}: {children: React.ReactNode}) => ( + {children} + ), + } + ) + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [textAt(7152)], + moreToLoad: true, + scrollDirection: 'back', + }) + }) + const rpc = jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + calls++ + await Promise.resolve() + // Someone hits jump-to-recent while the first page is in flight. + if (calls === 1) { + act(() => { + result.current.actions.messagesClear() + }) + } + p.onFullThread?.( + JSON.stringify({messages: tombstones(7151, 7052), pagination: {last: false, num: 100}}) + ) + return undefined as never + }) + loadBack(contextFor(result.current.store, result.current.actions)) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(1) + }) + + test('does not reload an initial load', async () => { + const rpc = mockWalkingBack(6152) + drive(seededContext(), { + allowMarkAsRead: true, + numberOfMessagesToLoad: 100, + reason: 'focused', + retryCount: 0, + scrollDirection: 'none', + }) + await act(async () => { + await flushPromises() + }) + expect(rpc).toHaveBeenCalledTimes(1) + }) +}) + +describe('only a pass that can account for a whole window reconciles', () => { + const page = (from: number, to: number) => + Array.from({length: from - to + 1}, (_, i) => ({ + placeholder: {hidden: false, messageID: T.Chat.numberToMessageID(from - i)}, + state: T.RPCChat.MessageUnboxedState.placeholder, + })) + + const mockPasses = (cached: string, full: string) => + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onCachedThread?.(cached) + p.onFullThread?.(full) + return undefined as never + }) + + // Whether the last pass of a load was the one that reconciles. What gets pruned is the store's + // business - addMessagesToThreadState fills the carried set itself - so these tests only check + // which passes are allowed to ask for it; the thread-context suite covers the pruning. + const renderRecording = () => { + const {result} = renderHook( + () => ({actions: useConversationThreadActions(), store: useConversationThreadStore()}), + { + wrapper: ({children}: {children: React.ReactNode}) => ( + {children} + ), + } + ) + const applyThreadLoad = jest.spyOn(result.current.actions, 'applyThreadLoad') + const load = () => + runThreadWindowLoad({ + actions: result.current.actions, + conversationIDKey: convID, + gate: {nextLoadID: 0, refillOwner: undefined}, + load: { + allowMarkAsRead: true, + numberOfMessagesToLoad: 100, + reason: 'focused', + retryCount: 0, + scrollDirection: 'none', + }, + isMounted: () => true, + onThreadLoadStatus: () => {}, + reload: () => {}, + store: result.current.store, + }) + const prunedOnLastPass = () => + applyThreadLoad.mock.calls.at(-1)?.[0].reconcile?.prune + return {applyThreadLoad, load, prunedOnLastPass} + } + + test('reconciles on a full pass that followed an empty cached one', async () => { + // First open after a db nuke: PullLocalOnly finds nothing, but its collector suppresses the + // miss and a cached pass is sent anyway, carrying no messages. INCREMENTAL against an empty + // local thread filters nothing out, so the full pass really is the whole window - and only a + // whole window may prune the stale ordinals a cache repair left behind. + mockPasses( + JSON.stringify({messages: null, pagination: {last: false, num: 100}}), + JSON.stringify({messages: page(7153, 7152), pagination: {last: false, num: 100}}) + ) + const {load, prunedOnLastPass} = renderRecording() + load() + await act(async () => { + await flushPromises() + }) + + expect(prunedOnLastPass()).toBe(true) + }) + + test('does not reconcile when the service never reported a cached pass', async () => { + // The service records the cached thread as sent before it marshals it, so a failure there + // leaves the full pass INCREMENTAL against a pass we were never shown. The cached callback + // firing - with a thread, or with the nil a cold cache sends - is the only sign we get that + // this did not happen. + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onFullThread?.(JSON.stringify({messages: page(7153, 7150), pagination: {last: false, num: 100}})) + return undefined as never + }) + const {load, prunedOnLastPass} = renderRecording() + load() + await act(async () => { + await flushPromises() + }) + + expect(prunedOnLastPass()).toBe(false) + }) + + test('reconciles on the full pass of a warm-cache load, against both passes', async () => { + // The warm-cache sequence: the cached pass carries the page and the full pass behind it is + // INCREMENTAL, only what changed. Neither is a window on its own - but INCREMENTAL walks the + // authoritative window and omits only what the cached pass already carried unchanged, so the + // two together are that window, and the range spans both. Judging the full pass alone would + // give up pruning on every conversation the cache is warm for, which is all of them after the + // first open. + mockPasses( + JSON.stringify({messages: page(7153, 7052), pagination: {last: false, num: 100}}), + JSON.stringify({messages: page(7153, 7153), pagination: {last: false, num: 100}}) + ) + const {load, prunedOnLastPass} = renderRecording() + load() + await act(async () => { + await flushPromises() + }) + + expect(prunedOnLastPass()).toBe(true) + }) + + test('does not reconcile when a cached pass arrived but another load owned the window', async () => { + // The gate-owner guard is the one guard that can turn a cached pass away and still let the full + // pass behind it through: the owner drops the gate in between. The service counts that cached + // pass as sent either way, so the full pass is INCREMENTAL - a handful of changed messages - + // and a span built from those alone covers every row between them with nothing recorded as + // present. That is not a stale-row cleanup, it is deleting the thread. + const {result} = renderHook( + () => ({actions: useConversationThreadActions(), store: useConversationThreadStore()}), + { + wrapper: ({children}: {children: React.ReactNode}) => ( + {children} + ), + } + ) + act(() => { + result.current.actions.messagesClear() + }) + const applyThreadLoad = jest.spyOn(result.current.actions, 'applyThreadLoad') + // Another load got to the cleared window first and still holds it. + const gate: {nextLoadID: number; refillOwner: number | undefined} = {nextLoadID: 1, refillOwner: 0} + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + p.onCachedThread?.(JSON.stringify({messages: page(7153, 7052), pagination: {last: false, num: 100}})) + // The load that owned the gate settles here, so the full pass is no longer refused. + gate.refillOwner = undefined + p.onFullThread?.(JSON.stringify({messages: page(7153, 7150), pagination: {last: false, num: 100}})) + return undefined as never + }) + runThreadWindowLoad({ + actions: result.current.actions, + conversationIDKey: convID, + gate, + load: { + allowMarkAsRead: true, + numberOfMessagesToLoad: 100, + reason: 'focused', + retryCount: 0, + scrollDirection: 'none', + }, + isMounted: () => true, + onThreadLoadStatus: () => {}, + reload: () => {}, + store: result.current.store, + }) + await act(async () => { + await flushPromises() + }) + + expect(applyThreadLoad).not.toHaveBeenCalled() + }) +}) + +// End to end: a load reconciles the window it refills. Which passes may ask to reconcile is +// covered above; these are about what the store then does with the answer. +describe('a load reconciles the window it refills', () => { + test('a warm-cache load prunes against both passes, not either one alone', async () => { + // Regression: once the service has sent a cached thread it switches the full response to + // INCREMENTAL, so the full pass only carries what changed. Treating either pass on its own as + // authoritative deleted real messages that were still in the thread. The two together are the + // window - INCREMENTAL walks it and omits only what the cached pass already carried - so the + // range spans both, and everything inside it that either pass carried survives. + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) + const ids = [301, 302, 303, 304].map(T.Chat.numberToMessageID) + // The cache holds the older three; only 304 changed, so that is all the full pass carries. The + // span is what makes this the dangerous shape: a range of [301..304] computed from the full pass + // alone covers 302 and 303, which are absent from it and would be pruned. + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({thread: threadJSON(ids.slice(0, 3))}) + await Promise.resolve() + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({thread: threadJSON([ids[3]!])}) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + // Seed a settled four-message window the way a whole-window full pass would. + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: ids.map(id => + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id, + ordinal: T.Chat.numberToOrdinal(T.Chat.messageIDToNumber(id)), + outboxID: undefined, + text: new HiddenString(`m${id}`), + timestamp: 100, + }) + ), + moreToLoad: false, + scrollDirection: 'none', + }) + }) + expect(result.current.window.ordinals).toEqual([301, 302, 303, 304]) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'test'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.window.ordinals).toEqual([301, 302, 303, 304]) + }) + + test('a warm-cache load does not prune a message sitting on its outbox ordinal', async () => { + // A message you sent keeps the fractional ordinal it had in the outbox, so the ordinal it parses + // with - its server one - is not the ordinal it occupies. The prune walks the window, so what + // the passes delivered has to be recorded in the window's terms too; recording the parsed + // ordinal deletes the row it was meant to protect. + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) + const outboxID = T.Chat.stringToOutboxID('sent-1') + const sentOrdinal = T.Chat.numberToOrdinal(302.001) + + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({ + thread: JSON.stringify({ + messages: [ + makeValidTextUIMessage(T.Chat.numberToMessageID(301), 'm301'), + makeValidTextUIMessage(T.Chat.numberToMessageID(302), 'm302'), + makeValidTextUIMessage(T.Chat.numberToMessageID(303), 'mine', 'sent-1'), + ], + pagination: {last: true, next: '', num: 100, previous: ''}, + }), + }) + await Promise.resolve() + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: JSON.stringify({ + messages: [makeValidTextUIMessage(T.Chat.numberToMessageID(301), 'm301 edited')], + pagination: {last: true, next: '', num: 100, previous: ''}, + }), + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + // The window as it stands after the send settled: the message is at its outbox ordinal, indexed + // under the server ID the service will send it back as. + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: [ + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id: T.Chat.numberToMessageID(301), + ordinal: T.Chat.numberToOrdinal(301), + outboxID: undefined, + text: new HiddenString('m301'), + timestamp: 100, + }), + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id: T.Chat.numberToMessageID(302), + ordinal: T.Chat.numberToOrdinal(302), + outboxID: undefined, + text: new HiddenString('m302'), + timestamp: 100, + }), + Message.makeMessageText({ + author: 'testuser', + conversationIDKey: convID, + id: T.Chat.numberToMessageID(303), + ordinal: sentOrdinal, + outboxID, + text: new HiddenString('mine'), + timestamp: 100, + }), + ], + moreToLoad: false, + scrollDirection: 'none', + }) + }) + expect(result.current.window.ordinals).toEqual([301, 302, sentOrdinal]) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'test'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.window.ordinals).toEqual([301, 302, sentOrdinal]) + }) + + test('a full pass that changed nothing still reconciles the window', async () => { + // The ordinary warm reload: the cached pass is the window and the INCREMENTAL full pass behind it + // carries nothing at all, because nothing changed. That is still an authoritative answer about + // the span, so a row the service no longer has is still a ghost - skipping the prune for want of + // messages to add leaves it on screen until the conversation is reopened. + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) + const ids = [301, 302, 303].map(T.Chat.numberToMessageID) + + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({ + thread: JSON.stringify({ + messages: [ids[0]!, ids[2]!].map(id => makeValidTextUIMessage(id, `m${id}`)), + pagination: {last: true, next: '', num: 100, previous: ''}, + }), + }) + await Promise.resolve() + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({ + thread: JSON.stringify({messages: null, pagination: {last: true, next: '', num: 100, previous: ''}}), + }) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: ids.map(id => + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id, + ordinal: T.Chat.numberToOrdinal(T.Chat.messageIDToNumber(id)), + outboxID: undefined, + text: new HiddenString(`m${id}`), + timestamp: 100, + }) + ), + moreToLoad: false, + scrollDirection: 'none', + }) + }) + expect(result.current.window.ordinals).toEqual([301, 302, 303]) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'test'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.window.ordinals).toEqual([301, 303]) + }) + + test('a warm-cache load still prunes a row neither pass carries', async () => { + // The other half of the same rule: a row inside the range that neither pass returned is a ghost - + // a cache repair left it behind, or it was deleted while we were away - and reconciling it away + // is what the range is for. Gating on a full pass with no cached one before it would have given + // this up for every conversation the cache is warm for. + useConfigState.setState({loggedIn: true}) + jest.spyOn(Common, 'isUserActivelyLookingAtThisThread').mockReturnValue(true) + jest.spyOn(T.RPCChat, 'localMarkAsReadLocalRpcPromise').mockResolvedValue({offline: false}) + const ids = [301, 302, 303, 304].map(T.Chat.numberToMessageID) + // 303 is in neither pass, and it sits inside the span the two of them cover. + jest.spyOn(T.RPCChat, 'localGetThreadNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatThreadCached']?.({thread: threadJSON([ids[0]!, ids[1]!])}) + await Promise.resolve() + p.incomingCallMap['chat.1.chatUi.chatThreadFull']?.({thread: threadJSON([ids[3]!])}) + await Promise.resolve() + return {offline: false} + }) + const {result} = renderWindow() + + act(() => { + result.current.actions.applyThreadLoad({ + centered: false, + enableActiveMarkRead: false, + messages: ids.map(id => + Message.makeMessageText({ + author: 'alice', + conversationIDKey: convID, + id, + ordinal: T.Chat.numberToOrdinal(T.Chat.messageIDToNumber(id)), + outboxID: undefined, + text: new HiddenString(`m${id}`), + timestamp: 100, + }) + ), + moreToLoad: false, + scrollDirection: 'none', + }) + }) + expect(result.current.window.ordinals).toEqual([301, 302, 303, 304]) + + act(() => { + result.current.requestWindow({anchor: 'newest', reason: 'test'}) + }) + await act(async () => { + await flushPromises() + }) + + expect(result.current.window.ordinals).toEqual([301, 302, 304]) + }) +}) diff --git a/shared/chat/conversation/thread-window.tsx b/shared/chat/conversation/thread-window.tsx new file mode 100644 index 000000000000..c06c69a44577 --- /dev/null +++ b/shared/chat/conversation/thread-window.tsx @@ -0,0 +1,750 @@ +import * as Message from '@/constants/chat/message' +import * as Meta from '@/constants/chat/meta' +import * as React from 'react' +import * as Strings from '@/constants/strings' +import * as T from '@/constants/types' +import logger from '@/logger' +import throttle from 'lodash/throttle' +import {clearChatTimeCache} from '@/util/timestamp' +import {getInboxConversationParticipants, unboxRows, updateInboxConversationMeta} from '@/chat/inbox/metadata' +import {ignorePromise} from '@/constants/utils' +import {loadThreadNonblock, threadLoadReasonToRPCReason} from './thread-rpc' +import {navigateToInbox} from '@/constants/router' +import {persistRoute} from '@/util/storeless-actions' +import {RPCError} from '@/util/errors' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useShallow} from '@/util/zustand' +import {useThreadStaleReloadListeners} from './thread-engine' +import {useUsersState} from '@/stores/users' +import type {ThreadLoadReconcile} from './thread-message-state' +import { + getCurrentUser, + getLastOrdinalFromSnapshot, + getMeta, + scrollDirectionToPagination, +} from './thread-load' +import { + type ConversationThreadActions, + type ConversationThreadState, + type ScrollDirection, + useConversationThreadActions, + useConversationThreadSelector, + useConversationThreadStore, +} from './thread-context' + +export const numMessagesOnInitialLoad = isMobile ? 20 : 100 +export const numMessagesOnScrollback = 100 +// How far the no-new-ordinals back-page chain will walk on its own before handing the thread back to +// the reader. See the reload block in runThreadLoad. +export const maxBackPageReloads = 10 + +const emptyOrdinals: ReadonlyArray = [] + +const emptyParticipantInfo: T.Chat.ParticipantInfo = { + all: [], + contactName: new Map(), + name: [], +} + +// Where the window the caller wants is anchored. +// - 'newest': the newest page. A 'jump to recent' reason drops the window it replaces first, since +// the newest page is disjoint from wherever a scrolled-back reader was. +// - 'older' / 'newer': one more page past the corresponding edge of the window we hold. +// - {centeredOn}: a window around one message, which always replaces the window we hold. +export type WindowAnchor = 'newest' | 'older' | 'newer' | {centeredOn: T.Chat.MessageID} + +export type WindowRequest = {anchor: WindowAnchor; reason: string} +export type RequestWindow = (p: WindowRequest) => void + +export type ThreadWindow = { + generation: number + loaded: boolean + moreToLoadBack: boolean + moreToLoadForward: boolean + ordinals: ReadonlyArray +} + +// Everything a load needs once requestWindow has turned an anchor into one. Not exported: callers +// name a place in the thread, the module decides how to fetch it. +type WindowLoad = { + allowMarkAsRead: boolean + centeredOn?: T.Chat.MessageID + numberOfMessagesToLoad: number + reason: string + // The oldest message ID the previous attempt saw, carried by the empty-back-page reload below. + // Each reload must reach strictly further back than that, which is what stops it looping. + retryBelowMessageID?: T.Chat.MessageID + // How many times the back-page reload has already chained. See maxBackPageReloads. + retryCount: number + scrollDirection: ScrollDirection +} + +// The one record arbitrating which response may become the loaded window, alongside the store's +// `generation`. +// +// `generation` moves on every clear and on a conversation change, so a response fetched against a +// window that no longer exists is refused. It cannot separate two loads issued after the SAME +// clear, and the second one is not hypothetical: a ChatThreadsStale reload fires with an anchor of +// 'newest' and fetches a region the clear never asked for. So the refill after a clear is also +// owned - first claim wins, and only the owner may refill or release - and `nextLoadID` names the +// loads for that comparison. Only ever compared for equality, never ordered. +type WindowGate = { + nextLoadID: number + refillOwner: number | undefined +} + +const noThreadLoadStatus = T.RPCChat.UIChatThreadStatusTyp.none + +const ThreadLoadStatusContext = React.createContext(noThreadLoadStatus) +ThreadLoadStatusContext.displayName = 'ThreadLoadStatusContext' + +const missingRequestWindow: RequestWindow = () => { + throw new Error('Missing ConversationThreadWindowProvider in the tree') +} + +const RequestWindowContext = React.createContext(missingRequestWindow) +RequestWindowContext.displayName = 'RequestWindowContext' + +export const useThreadLoadStatus = () => React.useContext(ThreadLoadStatusContext) + +export const useRequestWindow = () => React.useContext(RequestWindowContext) + +// The loaded window, as everything that renders it needs to see it. `generation` is the identity of +// the window itself: it moves when the window is dropped and reloaded, which is the only time a +// virtualized list has to forget the layout it measured. +export const useThreadWindow = (): ThreadWindow => + useConversationThreadSelector( + useShallow((s: ConversationThreadState) => ({ + generation: s.generation, + loaded: s.loaded, + moreToLoadBack: s.moreToLoadBack, + moreToLoadForward: s.moreToLoadForward, + ordinals: s.messageOrdinals ?? emptyOrdinals, + })) + ) + +// Repeat gate for the two scroll edges. The list re-fires its edge callbacks while the same window +// is on screen, so a request that names the same ordinal count as the last one is only let through +// once the previous one has had time to answer. +const makeScrollRepeatGate = () => { + let lastNumOrdinals = 0 + let lastTime = 0 + return (numOrdinals: number) => { + const now = Date.now() + if (numOrdinals !== lastNumOrdinals) { + lastNumOrdinals = numOrdinals + lastTime = now + return true + } + const ok = now - lastTime > 500 + if (ok) { + lastNumOrdinals = numOrdinals + lastTime = now + } + return ok + } +} + +type ThreadWindowProviderProps = React.PropsWithChildren<{ + allowMarkReadOnLoad?: boolean + id: T.Chat.ConversationIDKey + skipThreadLoadOnSelection?: boolean +}> + +export const ConversationThreadWindowProvider = (p: ThreadWindowProviderProps) => { + const {allowMarkReadOnLoad = true, children, id, skipThreadLoadOnSelection = false} = p + const [initialSkipThreadLoadOnSelection] = React.useState(skipThreadLoadOnSelection) + const actions = useConversationThreadActions() + const store = useConversationThreadStore() + + const currentIDRef = React.useRef(id) + React.useLayoutEffect(() => { + currentIDRef.current = id + }, [id]) + // A load that outlives its provider must not keep chaining reloads or reporting status. The + // generation cannot express this on its own: in StrictMode the provider mounts, unmounts and + // remounts with the same id, and moving the generation there would discard the first RPC's + // callbacks while the daemon deduplicates the second one and sends no data. + const mountedRef = React.useRef(true) + React.useEffect(() => { + mountedRef.current = true + return () => { + mountedRef.current = false + } + }, []) + React.useEffect(() => { + return () => { + // Only when the conversation actually changed, for the StrictMode reason above. + if (currentIDRef.current !== id) { + actions.bumpWindowGeneration() + } + } + }, [actions, id]) + + const [statusState, setStatusState] = React.useState<{ + conversationIDKey: T.Chat.ConversationIDKey + status: T.RPCChat.UIChatThreadStatusTyp + }>(() => ({conversationIDKey: id, status: noThreadLoadStatus})) + + const getConversationIDKey = React.useEffectEvent(() => id) + const isMounted = React.useEffectEvent(() => mountedRef.current) + + const onThreadLoadStatus = React.useEffectEvent( + (conversationIDKey: T.Chat.ConversationIDKey, status: T.RPCChat.UIChatThreadStatusTyp) => { + if (conversationIDKey !== id) { + return + } + setStatusState(previous => + previous.conversationIDKey === conversationIDKey && previous.status === status + ? previous + : {conversationIDKey, status} + ) + } + ) + + // The gate and the load throttle live together, in one object built once, because the reload + // chain has to come back through the throttle: a long run of tombstones would otherwise issue its + // pages back to back with no pacing. + const [loader] = React.useState(() => { + const gate: WindowGate = {nextLoadID: 0, refillOwner: undefined} + const runNow = (load: WindowLoad) => { + runThreadWindowLoad({ + actions, + conversationIDKey: getConversationIDKey(), + gate, + load, + isMounted, + onThreadLoadStatus, + reload: loadWindow, + store, + }) + } + const throttled = throttle(runNow, 500) + // The clearing requests (a centered jump, jump to recent) bypass the throttle: they empty the + // window before loading, and a trailing-edge throttle would drop the reload that refills it. + function loadWindow(load: WindowLoad) { + if (load.centeredOn || load.reason === 'jump to recent') { + throttled.cancel() + runNow(load) + } else { + throttled(load) + } + } + return { + cancel: () => { + throttled.cancel() + }, + loadWindow, + releaseRefillOwner: () => { + gate.refillOwner = undefined + }, + } + }) + const {loadWindow} = loader + React.useEffect(() => () => loader.cancel(), [loader]) + + // One gate per edge: a back page and a forward page carrying the same ordinal count are two + // different requests, and sharing a gate would let either swallow the other. + const [olderGate] = React.useState(makeScrollRepeatGate) + const [newerGate] = React.useState(makeScrollRepeatGate) + + // Drop the window before reloading. Both callers reload a region disjoint from the one being + // dropped - a centered jump an arbitrary one, jump to recent the newest page - so merging the two + // would leave ordinals with a hole through the middle. + const clearWindow = React.useEffectEvent(() => { + loader.releaseRefillOwner() + actions.messagesClear() + }) + + const onRequestWindow = React.useEffectEvent((request: WindowRequest) => { + const {anchor, reason} = request + if (typeof anchor === 'object') { + clearWindow() + loadWindow({ + allowMarkAsRead: true, + centeredOn: anchor.centeredOn, + numberOfMessagesToLoad: numMessagesOnInitialLoad, + reason, + retryCount: 0, + scrollDirection: 'none', + }) + return + } + const snapshot = store.getState() + const numOrdinals = snapshot.messageOrdinals?.length ?? 0 + switch (anchor) { + case 'newest': { + if (reason === 'jump to recent') { + actions.setMarkReadBlocked(false) + clearWindow() + } + loadWindow({ + // Only the automatic loads this module issues itself are held back by the provider's + // allowMarkReadOnLoad; a reader asking for the newest page has asked to be caught up. + allowMarkAsRead: true, + numberOfMessagesToLoad: numMessagesOnInitialLoad, + reason, + retryCount: 0, + scrollDirection: 'none', + }) + return + } + case 'older': { + if (!snapshot.moreToLoadBack) { + logger.info('requestWindow: bail: scrolling back and at the end') + return + } + if (!numOrdinals || !olderGate(numOrdinals)) { + return + } + loadWindow({ + allowMarkAsRead: true, + numberOfMessagesToLoad: numMessagesOnScrollback, + reason, + retryCount: 0, + scrollDirection: 'back', + }) + return + } + case 'newer': { + if (!snapshot.moreToLoadForward) { + return + } + if (!numOrdinals || !newerGate(numOrdinals)) { + return + } + loadWindow({ + allowMarkAsRead: true, + numberOfMessagesToLoad: numMessagesOnScrollback, + reason, + retryCount: 0, + scrollDirection: 'forward', + }) + return + } + } + }) + + // Stable identity: the context value must not change every render, or every consumer re-renders. + const [requestWindow] = React.useState( + () => (request: WindowRequest) => onRequestWindow(request) + ) + + const reloadStaleThread = React.useEffectEvent(() => { + loadWindow({ + allowMarkAsRead: allowMarkReadOnLoad, + numberOfMessagesToLoad: numMessagesOnInitialLoad, + reason: 'got stale', + retryCount: 0, + scrollDirection: 'none', + }) + }) + useThreadStaleReloadListeners(id, reloadStaleThread) + + const selectConversation = React.useEffectEvent(() => { + clearChatTimeCache() + unboxRows([id]) + const username = useCurrentUserState.getState().username + const participantInfo = getInboxConversationParticipants(id) ?? emptyParticipantInfo + const otherParticipants = Meta.getRowParticipants(participantInfo, username || '') + if (otherParticipants.length === 1) { + const otherUsername = otherParticipants[0] || '' + if (otherUsername && !otherUsername.includes('@')) { + useUsersState.getState().dispatch.getBio(otherUsername) + } + } + if (initialSkipThreadLoadOnSelection) { + return + } + loadWindow({ + allowMarkAsRead: allowMarkReadOnLoad, + numberOfMessagesToLoad: numMessagesOnInitialLoad, + reason: 'focused', + retryCount: 0, + scrollDirection: 'none', + }) + }) + React.useEffect(() => { + logger.info( + `ConversationThreadWindowProvider: selecting thread: ${id} skipThreadLoad=${initialSkipThreadLoadOnSelection}` + ) + selectConversation() + }, [id, initialSkipThreadLoadOnSelection]) + + const status = statusState.conversationIDKey === id ? statusState.status : noThreadLoadStatus + + return ( + + {children} + + ) +} + +// The whole of the arbitration, in one place. Split out of the provider so the rules read as one +// sequence rather than as a component body, and so a test can drive them without React. +export const runThreadWindowLoad = (p: { + actions: ConversationThreadActions + conversationIDKey: T.Chat.ConversationIDKey + gate: WindowGate + load: WindowLoad + isMounted: () => boolean + onThreadLoadStatus: ( + conversationIDKey: T.Chat.ConversationIDKey, + status: T.RPCChat.UIChatThreadStatusTyp + ) => void + reload: (load: WindowLoad) => void + store: {getState: () => ConversationThreadState} +}) => { + const {actions, conversationIDKey, gate, isMounted, load, onThreadLoadStatus, reload, store} = p + if (!T.Chat.isValidConversationIDKey(conversationIDKey)) { + return + } + const { + allowMarkAsRead, + centeredOn, + numberOfMessagesToLoad, + reason, + retryBelowMessageID, + retryCount, + scrollDirection, + } = load + + const f = async () => { + const loadStartedSnapshot = store.getState() + const generationAtLoadStart = loadStartedSnapshot.generation + // Whether the window this load was fetched against is still the loaded one. The generation + // moves on every clear and on a conversation change, so a response that would repopulate a + // window the reader has already left - jump to recent, a centered jump, a different + // conversation - is refused rather than merged into whatever replaced it. + const ownsTheWindow = () => store.getState().generation === generationAtLoadStart + const isCurrentLoad = () => isMounted() && ownsTheWindow() + + // applyThreadLoad drops the window gate when a load refills the window, but a load can end + // without ever applying: offline, scchatnotinteam, a response carrying no thread, or a bail + // before the RPC is even made. Left alone the gate would keep dropping notifications for the + // life of the provider, which is a thread that silently stops receiving messages. + // + // Claimed here, before the first await, rather than when a response arrives: both clearing + // requests bypass the load throttle and call in synchronously, so the reload the clear issued + // is always the first to get here, and a load that ends without ever applying still has to be + // the one that releases. + const loadID = gate.nextLoadID++ + if (loadStartedSnapshot.windowCleared && gate.refillOwner === undefined) { + gate.refillOwner = loadID + } + // Not gated on the provider still being mounted: an unmounted load is exactly the one with + // nothing coming after it, so leaving the gate up would strand the window for good. + const releaseWindowGate = () => { + if (!ownsTheWindow() || !store.getState().windowCleared) { + return + } + // An unclaimed gate is released by whoever settles first: nothing claimed it, so there is no + // reload in flight to protect, and leaving it up would strand the thread. + if (gate.refillOwner !== undefined && gate.refillOwner !== loadID) { + return + } + gate.refillOwner = undefined + actions.releaseWindowGate() + } + // Every bail from here on releases: the clear issues its reload synchronously, so if that + // reload is the one bailing there is nothing else coming to take the gate down, and the thread + // stops receiving messages for good. + if (!isCurrentLoad()) { + logger.info('requestWindow: bail: stale thread load') + releaseWindowGate() + return + } + + const currentMeta = getMeta(conversationIDKey) + if (currentMeta.membershipType === 'youAreReset' || currentMeta.rekeyers.size > 0) { + logger.info('requestWindow: bail: we are reset') + releaseWindowGate() + return + } + const loadStartedLiveUpdateVersion = loadStartedSnapshot.liveUpdateVersion + // A refresh of a window we already hold must not overwrite what a notification streamed into it + // while the RPC was out. Kept apart from the generation on purpose: this is content churn + // inside one window, not a new window, and folding the two would remount the thread list on + // every incoming message. + const protectLoadedFocusRefresh = + loadStartedSnapshot.loaded && + scrollDirection === 'none' && + !centeredOn && + (reason === 'focused' || reason === 'tab selected') + logger.info( + `requestWindow: calling rpc convo: ${conversationIDKey} num: ${numberOfMessagesToLoad} reason: ${reason}` + ) + + const loadingKey = Strings.waitingKeyChatThreadLoad(conversationIDKey) + // What this load has put in the window, filled in by addMessagesToThreadState as each pass + // applies. Once the service has sent a cached thread it switches the full response to + // INCREMENTAL, which walks the authoritative window and sends only the messages that cached + // pass did not already carry unchanged (mergeLocalRemoteThread in go/chat/uithreadloader.go, + // where localSentThread is that exact pass). Neither pass is a whole window on its own, so the + // two are gathered here and the last one reconciles against the both of them. + const carried = new Set() + // A load is all or nothing. Once one of its passes is turned away, the rest of them are too: + // the service filters each pass against what it has already sent this load, so the ones that + // follow a refused pass are a subset of a window we never took, and both ways of using them + // are wrong. Merging one into whatever refilled the window in the meantime is the disjoint + // window this whole invariant exists to prevent; reconciling against one takes out every row + // between the few messages it happens to carry. + let refusedAPass = false + // Whether the service's cached goroutine reported at all - with a thread, or with the nil it + // sends when the local cache had nothing. It is the only evidence the client gets that the + // full pass was not filtered behind our back: the service records the cached thread as sent + // before it marshals it, so a marshal failure there leaves us with an INCREMENTAL full pass + // and no sign of the pass it was filtered against (LoadNonblock in + // go/chat/uithreadloader.go). No report, no reconciling. + let sawCachedReport = false + // The reload below is judged against the whole load, not one pass of it. A warm-cache load + // delivers the page on the cached pass and then an INCREMENTAL full pass carrying only what + // changed, so measuring the full pass alone says "added nothing" for a perfectly good page. + // Measuring from before either pass tells the two apart: a page of real messages moves this, + // a page of tombstones does not, wherever it arrived. + const floorAtLoadStart = loadStartedSnapshot.messageOrdinals?.[0] + let oldestSeenThisLoad = Number.MAX_SAFE_INTEGER as T.Chat.MessageID + const onGotThread = (thread: string, why: string) => { + if (!thread) { + return + } + if (refusedAPass) { + logger.info(`requestWindow: pass ignored, an earlier one of this load was: ${why}`) + return + } + const refuse = (msg: string) => { + refusedAPass = true + logger.info(msg) + } + if (!isCurrentLoad()) { + refuse(`requestWindow: response ignored, the window it was fetched against is gone: ${why}`) + return + } + // The generation cannot separate two loads issued after the same clear, and the second one is + // not hypothetical: a ChatThreadsStale reload fetches the newest page, not the region the + // clear asked for. If it answers first it would fill the cleared window with that disjoint + // page and drop the gate, and the reload the clear issued would then merge its own page into + // the leftovers. While the gate is up only its owner may refill the window; once the owner + // settles the gate is down and everyone applies normally again. + const snapshotAtResponse = store.getState() + if ( + snapshotAtResponse.windowCleared && + gate.refillOwner !== undefined && + gate.refillOwner !== loadID + ) { + refuse(`requestWindow: response ignored, another load owns the window: ${why}`) + return + } + if (protectLoadedFocusRefresh && snapshotAtResponse.liveUpdateVersion !== loadStartedLiveUpdateVersion) { + refuse( + `requestWindow: stale response ignored after live update: ${why} reason=${reason} convID=${conversationIDKey}` + ) + return + } + + const {username, devicename} = getCurrentUser() + const {messages, pagination} = Message.parseUIMessagesJSON( + conversationIDKey, + thread, + username, + devicename, + () => getLastOrdinalFromSnapshot(store.getState()) + ) + const moreToLoad = pagination ? !pagination.last : true + const canMarkReadForThreadWindow = + allowMarkAsRead && + !centeredOn && + scrollDirection !== 'back' && + reason !== 'findNewestConversation' && + reason !== 'findNewestConversationFromLayout' + // Reconciling is only safe against a whole window, and a single pass is not one: the cached + // pass is whatever the local cache holds, gaps included, and the full pass behind it carries + // only what changed. The full pass is the last one, so it is the one that prunes - against + // everything both passes delivered. Waiting instead for a pass with no cached one before it + // would leave the stale-row cleanup running on cold caches only, which is where ghost rows + // are least likely to be: a reopened conversation is warm every time. + const reconcile: ThreadLoadReconcile | undefined = + scrollDirection === 'none' ? {carried, prune: why === 'full' && sawCachedReport} : undefined + for (const m of messages) { + if (m.id > 0 && m.id < oldestSeenThisLoad) { + oldestSeenThisLoad = m.id + } + } + if (mayJoinWindow(snapshotAtResponse, messages, scrollDirection)) { + actions.applyThreadLoad({ + centered: !!centeredOn, + disableActiveMarkRead: !allowMarkAsRead || !!centeredOn, + enableActiveMarkRead: canMarkReadForThreadWindow, + messages, + moreToLoad, + reconcile, + scrollDirection, + }) + // Only a pass that actually rendered something drops the gate. A cold cache sends an empty + // cached pass ahead of the full response, and a page can be all tombstones: dropping the + // gate on either would let a notification arriving before the real page install itself as + // the whole window and strand once that page lands. A load that ends without ever producing + // an ordinal releases the gate in its own finally instead. + if (renderedMessages(messages).length) { + gate.refillOwner = undefined + actions.releaseWindowGate() + } + } + const after = store.getState() + // A back page can be composed entirely of messages the thread will never render: a message + // superseded by a DELETE arrives as a hidden placeholder, becomes `deleted`, and addMessages + // drops it. The ordinal list is then identical to what it was, so the list never fires + // onStartReached again and scrollback stops even though the pager says there is more. Ask for + // the next page ourselves. + // + // The tombstones still carry message IDs, and each page reaches further back than the last, + // so requiring strict progress terminates: message IDs are finite and only ever decrease + // here. Strict progress alone is a weak bound though - a channel whose history was largely + // expunged has tens of thousands of them, which is minutes of paging off one gesture - so the + // chain also stops after maxBackPageReloads. Stopping is safe: the reader is still pinned at + // the top with an unchanged list, and scrolling away and back fires onStartReached again, + // which starts a fresh chain from wherever this one left off. + const floorAfter = after.messageOrdinals?.[0] + const windowGrewDownward = + floorAfter !== undefined && (floorAtLoadStart === undefined || floorAfter < floorAtLoadStart) + if ( + scrollDirection === 'back' && + // The full pass is the last one of a load, so by here the whole load has been applied. + why === 'full' && + moreToLoad && + // The floor, not the count: a page can add real messages while its `deleted` entries + // remove more from the window, which nets negative on a count but is real progress. + !windowGrewDownward && + oldestSeenThisLoad < (retryBelowMessageID ?? Number.MAX_SAFE_INTEGER) && + retryCount < maxBackPageReloads + ) { + logger.info( + `requestWindow: back page added no ordinals, reloading below ${oldestSeenThisLoad} (${ + retryCount + 1 + }/${maxBackPageReloads}): convID: ${conversationIDKey}` + ) + // Back through the throttled entry point, not straight into another load: a long run of + // tombstones would otherwise issue these back to back with no pacing. The throttle only + // ever drops a call that a later load supersedes, and that load extends the window or + // retries in turn. + // + // The delay has a cost: the next page comes from a cursor the daemon holds, not one we + // send. pgmode is SERVER (see thread-rpc), so `next` resolves against convPageStatus in the + // service, and any first-page request resets it (applyPagerModeOutgoing in + // go/chat/uithreadloader.go) - which every 'none' load is, stale and focus reloads + // included. One landing inside the throttle window makes this retry fetch near the top of + // the thread instead of the next page back. It fails closed rather than looping: + // oldestSeenThisLoad is then no lower than retryBelowMessageID, so the chain stops and the + // reader is left where another scroll gesture starts a fresh one. + // + // Sizing, for the same reason the chain is bounded at all: a full run is 11 sequential + // 100-message RPCs off one gesture, several seconds of paging with nothing visible moving. + reload({...load, retryBelowMessageID: oldestSeenThisLoad, retryCount: retryCount + 1}) + } + + if (canMarkReadForThreadWindow) { + actions.markThreadAsRead() + } + } + + const messageIDControl = centeredOn + ? {mode: T.RPCChat.MessageIDControlMode.centered, num: numberOfMessagesToLoad, pivot: centeredOn} + : null + const pagination = messageIDControl + ? null + : scrollDirectionToPagination(scrollDirection, numberOfMessagesToLoad) + try { + const results = await loadThreadNonblock({ + conversationIDKey, + messageIDControl, + onCachedThread: thread => { + sawCachedReport = true + onGotThread(thread, 'cached') + }, + onFullThread: thread => onGotThread(thread, 'full'), + onThreadStatus: status => { + logger.info( + `requestWindow: thread status received: convID: ${conversationIDKey} typ: ${status.typ}` + ) + if (isCurrentLoad()) { + onThreadLoadStatus(conversationIDKey, status.typ) + } + }, + pagination, + reason: threadLoadReasonToRPCReason(reason), + waitingKey: loadingKey, + }) + if (!isCurrentLoad()) { + return + } + updateInboxConversationMeta(conversationIDKey, {offline: results.offline}) + } catch (error) { + if (!isCurrentLoad()) { + return + } + if (error instanceof RPCError) { + logger.warn(`requestWindow: error: ${error.desc}`) + if (error.code === T.RPCGen.StatusCode.scchatnotinteam) { + // We're no longer in this conv's team. Clear the persisted last-route + // (ui.routeState2) so app startup doesn't keep restoring and reloading + // this conv, which would re-trigger this error on every launch. + persistRoute(true, true, () => useConfigState.getState().startup.loaded) + navigateToInbox(true, 'maybeKickedFromTeam') + } + if (error.code !== T.RPCGen.StatusCode.scteamreaderror) { + throw error + } + } + } finally { + releaseWindowGate() + } + } + + ignorePromise(f()) +} + +const renderedMessages = (messages: ReadonlyArray) => + messages.filter(m => m.conversationMessage !== false && m.type !== 'deleted') + +// Whether a page may join the window we hold, judged on what it carried rather than on the state of +// the window. +// +// A 'none' load fetches the newest page, and a window with more to load forward does not reach it. +// Merging the two leaves ordinals with a hole through the middle, and the window then reports +// itself as containing the latest message - which is the gap this whole invariant is about, +// arriving through a ChatThreadsStale reload while the reader sits on a search result. Both +// conditions are needed: a window that already reaches the newest message merges fine, and so does +// a page that overlaps what we hold, however far back the reader is. +const mayJoinWindow = ( + snapshot: ConversationThreadState, + messages: ReadonlyArray, + scrollDirection: ScrollDirection +) => { + const rendered = renderedMessages(messages) + const windowOrdinals = snapshot.messageOrdinals + const floor = windowOrdinals?.[0] + const ceiling = windowOrdinals?.[windowOrdinals.length - 1] + if ( + scrollDirection !== 'none' || + !rendered.length || + !snapshot.moreToLoadForward || + floor === undefined || + ceiling === undefined + ) { + return true + } + let lowest = Number.MAX_SAFE_INTEGER + let highest = Number.MIN_SAFE_INTEGER + for (const m of rendered) { + lowest = Math.min(lowest, m.ordinal) + highest = Math.max(highest, m.ordinal) + } + if (lowest > ceiling || highest < floor) { + logger.info( + `requestWindow: page ${lowest}-${highest} does not reach window ${floor}-${ceiling}, ignoring` + ) + return false + } + return true +} From 1a0a09b9ca8ea621bae79de2b474f64b9d572957 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 9 Sep 2026 19:21:08 -0400 Subject: [PATCH 2/3] fix(chat): a page that cannot join the window retires the load The refusal exists because a pass that does not join the loaded window is poisonous twice over: merging it produces the disjoint window, and reconciling against it takes out every row between the few messages it happens to carry. The disjoint pass was the one case that skipped the pass without arming it. That is enough on its own, because the passes are not independent. The service records the cached thread as sent before it marshals it, so `sawCachedReport` is already set by the time the pass is dropped - and the full pass behind a warm cache is INCREMENTAL, carrying only what changed. So the load continued with the prune armed and nothing in `carried` from the pass that was dropped, and the full pass reconciled a centered window against the handful of rows it held: everything between them deleted, an old window marked read on the way out. The test drives it the way it actually happens - a cached pass from newer than the window the reader is sitting on, then a full pass spanning it - and without the refusal the two rows between the span's ends are gone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RYPqG8z11HmGRn5MHSu89S --- .../chat/conversation/thread-window.test.tsx | 47 +++++++++++++++++++ shared/chat/conversation/thread-window.tsx | 46 ++++++++++-------- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/shared/chat/conversation/thread-window.test.tsx b/shared/chat/conversation/thread-window.test.tsx index e0c455fd0ac6..4d7725e6a4e3 100644 --- a/shared/chat/conversation/thread-window.test.tsx +++ b/shared/chat/conversation/thread-window.test.tsx @@ -966,6 +966,53 @@ describe('the window gate', () => { expect(context.store.getState().windowCleared).toBe(false) }) + test('a disjoint cached pass retires the load instead of letting the full pass prune', async () => { + // The cached pass is whatever the local cache holds and it can be a page from somewhere else + // entirely - here, newer messages while the reader sits on a centered window. Skipping just that + // pass is not enough: the service has already recorded that a cached thread was sent, so the + // INCREMENTAL full pass behind it arrives carrying only what changed and, with nothing in + // `carried` from the pass that was dropped, reconciles the whole window against those few rows - + // deleting everything between them and marking an old window read on the way past. + let sendFull: (() => void) | undefined + jest.spyOn(ThreadRpc, 'loadThreadNonblock').mockImplementation(async p => { + await Promise.resolve() + // Entirely above the loaded window: it cannot join. + p.onCachedThread?.(threadJSON([T.Chat.numberToMessageID(9000)], false)) + return new Promise(resolve => { + sendFull = () => { + p.onFullThread?.( + threadJSON([T.Chat.numberToMessageID(7150), T.Chat.numberToMessageID(7153)], false) + ) + resolve(undefined as never) + } + }) + }) + const {context, result} = renderContext() + act(() => { + result.current.actions.applyThreadLoad({ + centered: true, + enableActiveMarkRead: false, + messages: [textAt(7150), textAt(7151), textAt(7152), textAt(7153)], + moreToLoad: true, + scrollDirection: 'none', + }) + }) + const before = result.current.ordinals + + drive(context, newestLoad({reason: 'focused'})) + await act(async () => { + await flushPromises() + }) + await act(async () => { + sendFull?.() + await flushPromises() + }) + + // The window the reader is looking at is still all there: 7151 and 7152 sit inside the span the + // full pass would have reconciled against, and are exactly what the prune would have taken. + expect(result.current.ordinals).toEqual(before) + }) + test('an empty pass during a jump-to-recent gap leaves the gate up', async () => { // A cold cache sends a cached pass carrying no messages ahead of the full response. Dropping // the gate on it reopens the gap: a notification landing before the real page becomes the sole diff --git a/shared/chat/conversation/thread-window.tsx b/shared/chat/conversation/thread-window.tsx index c06c69a44577..bdde287ef78b 100644 --- a/shared/chat/conversation/thread-window.tsx +++ b/shared/chat/conversation/thread-window.tsx @@ -571,25 +571,33 @@ export const runThreadWindowLoad = (p: { oldestSeenThisLoad = m.id } } - if (mayJoinWindow(snapshotAtResponse, messages, scrollDirection)) { - actions.applyThreadLoad({ - centered: !!centeredOn, - disableActiveMarkRead: !allowMarkAsRead || !!centeredOn, - enableActiveMarkRead: canMarkReadForThreadWindow, - messages, - moreToLoad, - reconcile, - scrollDirection, - }) - // Only a pass that actually rendered something drops the gate. A cold cache sends an empty - // cached pass ahead of the full response, and a page can be all tombstones: dropping the - // gate on either would let a notification arriving before the real page install itself as - // the whole window and strand once that page lands. A load that ends without ever producing - // an ordinal releases the gate in its own finally instead. - if (renderedMessages(messages).length) { - gate.refillOwner = undefined - actions.releaseWindowGate() - } + if (!mayJoinWindow(snapshotAtResponse, messages, scrollDirection)) { + // A disjoint page is the case the refusal exists for, so it retires the whole load rather + // than just this pass. A warm cache sends the page on the cached pass and an INCREMENTAL + // full pass behind it carrying only what changed - so letting the load continue past a + // skipped cached pass leaves `sawCachedReport` set with nothing in `carried`, and an empty + // full pass then joins unopposed and prunes a centered window against the handful of rows + // it happens to hold, marking an old window read on the way out. + refuse(`requestWindow: pass ignored, it does not join the loaded window: ${why}`) + return + } + actions.applyThreadLoad({ + centered: !!centeredOn, + disableActiveMarkRead: !allowMarkAsRead || !!centeredOn, + enableActiveMarkRead: canMarkReadForThreadWindow, + messages, + moreToLoad, + reconcile, + scrollDirection, + }) + // Only a pass that actually rendered something drops the gate. A cold cache sends an empty + // cached pass ahead of the full response, and a page can be all tombstones: dropping the gate + // on either would let a notification arriving before the real page install itself as the + // whole window and strand once that page lands. A load that ends without ever producing an + // ordinal releases the gate in its own finally instead. + if (renderedMessages(messages).length) { + gate.refillOwner = undefined + actions.releaseWindowGate() } const after = store.getState() // A back page can be composed entirely of messages the thread will never render: a message From fbc976209949bc7e08427df1f8c1c521eb0782a8 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 9 Sep 2026 19:46:12 -0400 Subject: [PATCH 3/3] docs(chat): route the readme's thread loads through the window The architecture doc still sent thread loads through `loadMoreMessages` -> `loadConversationThreadMessages` in thread-load.tsx. Both are gone as of this branch: callers name a place in the thread with requestWindow and read it back with useThreadWindow, and runThreadWindowLoad issues the RPC behind that. thread-load.tsx is still there and still worth naming, but it no longer holds the RPC - what is left is what both the window and the row derivation draw on: meta and current-user lookups, exploding-mode-from-gregor, snapshot accessors, pagination sizing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RYPqG8z11HmGRn5MHSu89S --- shared/chat/readme.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/chat/readme.md b/shared/chat/readme.md index c77311b5c35e..df507deff25e 100644 --- a/shared/chat/readme.md +++ b/shared/chat/readme.md @@ -5,14 +5,14 @@ How chat works: Chat data is split across several focused stores instead of one global redux tree. Roughly: - **Inbox metadata** (`chat/inbox/metadata.tsx`, `useInboxMetadataState`) is the single owner of conversation `meta` (trustedState, snippet, participants pointer, draft, timestamp, etc.) and `participants`. All meta writes go through `metasReceived`, which version-gates each incoming meta against the currently stored one (`Meta.updateMeta`) so a stale/out-of-order update can't clobber newer data. Callers that already merged from the current meta (e.g. `updateInboxConversationMeta`, error metas, incremental inbox sync) pass `{force: true}` to bypass gating. Converters live in `constants/chat/meta.tsx` (`baseMetaFromUIItem` is the shared base used by the various `*ToConversationMeta` functions). -- **Per-conversation thread store** (`chat/conversation/thread-context.tsx`) is a vanilla zustand store created fresh per mounted `ConversationThreadProvider` and destroyed when the provider unmounts. It holds `messageMap`/`messageOrdinals`/`messageIDToOrdinal`/`messageTypeMap`/`pendingOutboxToOrdinal`, live `typing` (a `Set`), exploding mode, and payment/request/flip/unfurl maps. It reads conversation meta from the inbox metadata store rather than owning its own copy (`useThreadMeta`, `getMeta`). The module is split: `thread-engine.tsx` holds engine-notification handlers (`applyMessagesUpdatedToThread`, `applyIncomingMutationToThread`, etc.) and `thread-load.tsx` holds thread-load logic (RPC calls, exploding-mode-from-gregor, pagination sizing). +- **Per-conversation thread store** (`chat/conversation/thread-context.tsx`) is a vanilla zustand store created fresh per mounted `ConversationThreadProvider` and destroyed when the provider unmounts. It holds `messageMap`/`messageOrdinals`/`messageIDToOrdinal`/`messageTypeMap`/`pendingOutboxToOrdinal`, live `typing` (a `Set`), exploding mode, and payment/request/flip/unfurl maps. It reads conversation meta from the inbox metadata store rather than owning its own copy (`useThreadMeta`, `getMeta`). The module is split: `thread-engine.tsx` holds engine-notification handlers (`applyMessagesUpdatedToThread`, `applyIncomingMutationToThread`, etc.), `thread-window.tsx` owns the loaded window (the load RPC, the response gate, the clear-and-reload), and `thread-load.tsx` is what both draw on: meta and current-user lookups, exploding-mode-from-gregor, snapshot accessors, pagination sizing. - **Inbox rows are computed, not cached.** `chat/inbox/rows-state.tsx` exposes `useInboxRowSmall`/`useInboxRowBig`, which `useMemo` a display row from: inbox metadata (meta + participants), `chat/inbox/layout-state.tsx` (a memoized index built from the service's `UIInboxLayout`, used as a fallback for rows whose meta isn't trusted yet), `chat/inbox/badge-state.tsx` (badge/unread counts, fully replaced from each `BadgeState` RPC payload), and `chat/inbox/typing-state.tsx` (per-conversation typing username sets, merged in from `ChatTypingUpdate`). Merge precedence is one rule: meta wins whenever it's `trusted` or `error`; otherwise the layout row fills the gaps (snippet, draft, time, mute, name-split participants). - **Message conversion** lives in `constants/chat/message.tsx` (`uiMessageToMessage` converts a single RPC `UIMessage` to the internal `Message` type; `parseUIMessagesJSON` does the same for a JSON-stringified array, used for bulk thread-load ingestion). - **Orange line** (the "new messages" divider) is a small standalone store, `chat/conversation/orange-line-context.tsx` (`useExplicitOrangeLineState`), keyed by conversationIDKey -> `{ordinal, version}`. ## How data flows in -Engine notifications land in `shared/constants/init/shared.tsx`'s `_onEngineIncoming`, which calls `handleConvoEngineIncoming` (`chat/inbox/engine.tsx`) directly for chat-relevant action types. That function is the inbox-side router: it turns RPC notifications (`ChatConvUpdate`, `NewChatActivity`, `ChatTypingUpdate`, `ChatParticipantsInfo`, `ChatThreadsStale`, etc.) into calls against the metadata store (`metasReceived`, `metaReceivedError`, `updateInboxConversationMeta`), the typing store (`updateInboxTyping`), or an unbox request (`unboxRows`/`forceUnboxRowsForService`). Thread-specific engine events (message updates/mutations, reactions, attachments) are instead handled by `thread-engine.tsx`'s listeners, wired up per-conversation inside `thread-context.tsx` (`useThreadEngineListeners`) so they only run while that conversation's provider is mounted. Thread loads (initial, scrollback, centered, jump-to-recent) go through `loadMoreMessages` -> `loadConversationThreadMessages` in `thread-load.tsx`, which issues the RPC and calls back into the thread store's `applyThreadLoad`. +Engine notifications land in `shared/constants/init/shared.tsx`'s `_onEngineIncoming`, which calls `handleConvoEngineIncoming` (`chat/inbox/engine.tsx`) directly for chat-relevant action types. That function is the inbox-side router: it turns RPC notifications (`ChatConvUpdate`, `NewChatActivity`, `ChatTypingUpdate`, `ChatParticipantsInfo`, `ChatThreadsStale`, etc.) into calls against the metadata store (`metasReceived`, `metaReceivedError`, `updateInboxConversationMeta`), the typing store (`updateInboxTyping`), or an unbox request (`unboxRows`/`forceUnboxRowsForService`). Thread-specific engine events (message updates/mutations, reactions, attachments) are instead handled by `thread-engine.tsx`'s listeners, wired up per-conversation inside `thread-context.tsx` (`useThreadEngineListeners`) so they only run while that conversation's provider is mounted. Thread loads (initial, scrollback, centered, jump-to-recent) all go through one door: callers name a place in the thread with `requestWindow({anchor, reason})` (`thread-window.tsx`, anchor `'newest' | 'older' | 'newer' | {centeredOn}`) and read the window back with `useThreadWindow()`. Behind it, `runThreadWindowLoad` issues the RPC and calls back into the thread store's `applyThreadLoad`; which response may become the loaded window is arbitrated there by a private gate and the store's `generation`. ## Lifecycle