From f2006e15ba88dc6d4e41b0f7a88a363cd067c26e Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Sat, 25 Jul 2026 17:53:34 -0700 Subject: [PATCH] Optimize jump to unread to reduce flicker on large backlog load. --- app/models.py | 7 ++ app/repository/messages.py | 20 +++- frontend/src/App.tsx | 91 ++-------------- frontend/src/components/ConversationPane.tsx | 13 ++- frontend/src/components/MessageList.tsx | 46 ++++++-- frontend/src/hooks/useUnreadCounts.ts | 5 + frontend/src/test/conversationPane.test.tsx | 8 +- frontend/src/test/messageList.test.tsx | 70 ++++++++---- .../src/test/unreadMarkerBackfill.test.ts | 101 ------------------ frontend/src/test/useUnreadCounts.test.ts | 11 ++ frontend/src/types.ts | 2 + tests/test_api.py | 17 +++ 12 files changed, 163 insertions(+), 228 deletions(-) delete mode 100644 frontend/src/test/unreadMarkerBackfill.test.ts diff --git a/app/models.py b/app/models.py index 263077e..e72a261 100644 --- a/app/models.py +++ b/app/models.py @@ -960,6 +960,13 @@ class UnreadCounts(BaseModel): last_message_times: dict[str, int] = Field( default_factory=dict, description="Map of stateKey -> last message timestamp" ) + first_unread_ids: dict[str, int | None] = Field( + default_factory=dict, + description=( + "Map of stateKey -> id of the oldest unread message. Lets the client place " + "the unread divider (and jump to it) without paging back through history." + ), + ) last_read_ats: dict[str, int | None] = Field( default_factory=dict, description="Map of stateKey -> server-side last_read_at boundary" ) diff --git a/app/repository/messages.py b/app/repository/messages.py index cd66549..0338993 100644 --- a/app/repository/messages.py +++ b/app/repository/messages.py @@ -720,12 +720,19 @@ class MessageRepository: blocked_names: Display names whose messages should be excluded from counts. Returns: - Dict with 'counts', 'mentions', 'last_message_times', and 'last_read_ats' keys. + Dict with 'counts', 'mentions', 'last_message_times', 'last_read_ats', + and 'first_unread_ids' keys. """ counts: dict[str, int] = {} mention_flags: dict[str, bool] = {} last_message_times: dict[str, int] = {} last_read_ats: dict[str, int | None] = {} + # id of the oldest unread message per conversation. Rides the aggregate + # queries below via SQLite's bare-column rule: with exactly one MIN() in + # the query, bare columns come from the row that produced the minimum. + # Deliberately not MIN(id) — historical decryption inserts old messages + # with new ids, so id order and received_at order can disagree. + first_unread_ids: dict[str, int | None] = {} mention_token = f"@[{name}]" if name else None @@ -753,7 +760,9 @@ class MessageRepository: SUM(CASE WHEN ? <> '' AND INSTR(LOWER(m.text), LOWER(?)) > 0 THEN 1 ELSE 0 - END) > 0 as has_mention + END) > 0 as has_mention, + MIN(m.received_at) as first_unread_at, + m.id as first_unread_id FROM messages m JOIN channels c ON m.conversation_key = c.key WHERE m.type = 'CHAN' AND m.outgoing = 0 @@ -768,6 +777,7 @@ class MessageRepository: for row in rows: state_key = f"channel-{row['conversation_key']}" counts[state_key] = row["unread_count"] + first_unread_ids[state_key] = row["first_unread_id"] if mention_token and row["has_mention"]: mention_flags[state_key] = True @@ -779,7 +789,9 @@ class MessageRepository: SUM(CASE WHEN ? <> '' AND INSTR(LOWER(m.text), LOWER(?)) > 0 THEN 1 ELSE 0 - END) > 0 as has_mention + END) > 0 as has_mention, + MIN(m.received_at) as first_unread_at, + m.id as first_unread_id FROM messages m LEFT JOIN contacts ct ON m.conversation_key = ct.public_key WHERE m.type = 'PRIV' AND m.outgoing = 0 @@ -793,6 +805,7 @@ class MessageRepository: for row in rows: state_key = f"contact-{row['conversation_key']}" counts[state_key] = row["unread_count"] + first_unread_ids[state_key] = row["first_unread_id"] if mention_token and row["has_mention"]: mention_flags[state_key] = True @@ -841,6 +854,7 @@ class MessageRepository: "mentions": mention_flags, "last_message_times": last_message_times, "last_read_ats": last_read_ats, + "first_unread_ids": first_unread_ids, } @staticmethod diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 3289260..d658cae 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -27,19 +27,14 @@ import { RichPayloadProvider } from './contexts/RichPayloadContext'; import { usePush } from './contexts/PushSubscriptionContext'; import { messageContainsMention } from './utils/messageParser'; import { getStateKey } from './utils/conversationState'; -import type { - BulkCreateHashtagChannelsResult, - Channel, - Conversation, - Message, - RawPacket, -} from './types'; +import type { BulkCreateHashtagChannelsResult, Channel, Conversation, RawPacket } from './types'; import { CONTACT_TYPE_REPEATER, CONTACT_TYPE_ROOM } from './types'; import { shouldAutoFocusInput } from './utils/autoFocusInput'; interface ChannelUnreadMarker { channelId: string; - lastReadAt: number | null; + /** Id of the oldest unread message, straight from the server. */ + messageId: number | null; } interface NewMessagePrefillRequest { @@ -48,44 +43,6 @@ interface NewMessagePrefillRequest { nonce: number; } -interface UnreadBoundaryBackfillParams { - activeConversation: Conversation | null; - unreadMarker: ChannelUnreadMarker | null; - messages: Message[]; - messagesLoading: boolean; - loadingOlder: boolean; - hasOlderMessages: boolean; -} - -export function getUnreadBoundaryBackfillKey({ - activeConversation, - unreadMarker, - messages, - messagesLoading, - loadingOlder, - hasOlderMessages, -}: UnreadBoundaryBackfillParams): string | null { - if (activeConversation?.type !== 'channel') return null; - if (!unreadMarker || unreadMarker.channelId !== activeConversation.id) return null; - if (unreadMarker.lastReadAt === null) return null; - if (messagesLoading || loadingOlder || !hasOlderMessages || messages.length === 0) return null; - - const oldestLoadedMessage = messages.reduce( - (oldest, msg) => { - if (!oldest) return msg; - if (msg.received_at < oldest.received_at) return msg; - if (msg.received_at === oldest.received_at && msg.id < oldest.id) return msg; - return oldest; - }, - null as Message | null - ); - - if (!oldestLoadedMessage) return null; - if (oldestLoadedMessage.received_at <= unreadMarker.lastReadAt) return null; - - return `${activeConversation.id}:${unreadMarker.lastReadAt}:${oldestLoadedMessage.id}`; -} - export function App() { const quoteSearchOperatorValue = useCallback((value: string) => { return `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`; @@ -100,7 +57,6 @@ export function App() { const [bulkAddResult, setBulkAddResult] = useState(null); const [repeaterAutoLoginKey, setRepeaterAutoLoginKey] = useState(null); const [visibilityVersion, setVisibilityVersion] = useState(0); - const lastUnreadBackfillAttemptRef = useRef(null); const { notificationsSupported, notificationsPermission, @@ -355,7 +311,7 @@ export function App() { unreadCounts, mentions, lastMessageTimes, - unreadLastReadAts, + firstUnreadIds, recordMessageEvent, renameConversationState, removeConversationState, @@ -397,40 +353,10 @@ export function App() { } return { channelId: activeChannelId, - lastReadAt: unreadLastReadAts[getStateKey('channel', activeChannelId)] ?? null, + messageId: firstUnreadIds[getStateKey('channel', activeChannelId)] ?? null, }; }); - }, [activeConversation, unreadCounts, unreadLastReadAts]); - - useEffect(() => { - lastUnreadBackfillAttemptRef.current = null; - }, [activeConversation?.id, channelUnreadMarker?.channelId, channelUnreadMarker?.lastReadAt]); - - useEffect(() => { - const backfillKey = getUnreadBoundaryBackfillKey({ - activeConversation, - unreadMarker: channelUnreadMarker, - messages, - messagesLoading, - loadingOlder, - hasOlderMessages, - }); - - if (!backfillKey || lastUnreadBackfillAttemptRef.current === backfillKey) { - return; - } - - lastUnreadBackfillAttemptRef.current = backfillKey; - void fetchOlderMessages(); - }, [ - activeConversation, - channelUnreadMarker, - messages, - messagesLoading, - loadingOlder, - hasOlderMessages, - fetchOlderMessages, - ]); + }, [activeConversation, unreadCounts, firstUnreadIds]); const wsHandlers = useRealtimeAppState({ prevHealthRef, @@ -610,11 +536,12 @@ export function App() { messagesLoading, loadingOlder, hasOlderMessages, - unreadMarkerLastReadAt: + unreadMarkerMessageId: activeConversation?.type === 'channel' && channelUnreadMarker?.channelId === activeConversation.id - ? channelUnreadMarker.lastReadAt + ? channelUnreadMarker.messageId : undefined, + onNavigateToUnread: (messageId: number) => setTargetMessageId(messageId), targetMessageId, hasNewerMessages, loadingNewer, diff --git a/frontend/src/components/ConversationPane.tsx b/frontend/src/components/ConversationPane.tsx index 7e12c70..41a0818 100644 --- a/frontend/src/components/ConversationPane.tsx +++ b/frontend/src/components/ConversationPane.tsx @@ -50,7 +50,8 @@ interface ConversationPaneProps { messagesLoading: boolean; loadingOlder: boolean; hasOlderMessages: boolean; - unreadMarkerLastReadAt?: number | null; + unreadMarkerMessageId?: number | null; + onNavigateToUnread?: (messageId: number) => void; targetMessageId: number | null; hasNewerMessages: boolean; loadingNewer: boolean; @@ -137,7 +138,8 @@ export function ConversationPane({ messagesLoading, loadingOlder, hasOlderMessages, - unreadMarkerLastReadAt, + unreadMarkerMessageId, + onNavigateToUnread, targetMessageId, hasNewerMessages, loadingNewer, @@ -348,8 +350,11 @@ export function ConversationPane({ loading={messagesLoading} loadingOlder={loadingOlder} hasOlderMessages={hasOlderMessages} - unreadMarkerLastReadAt={ - activeConversation.type === 'channel' ? unreadMarkerLastReadAt : undefined + unreadMarkerMessageId={ + activeConversation.type === 'channel' ? unreadMarkerMessageId : undefined + } + onNavigateToUnread={ + activeConversation.type === 'channel' ? onNavigateToUnread : undefined } onDismissUnreadMarker={ activeConversation.type === 'channel' ? onDismissUnreadMarker : undefined diff --git a/frontend/src/components/MessageList.tsx b/frontend/src/components/MessageList.tsx index 351e0b5..3f28898 100644 --- a/frontend/src/components/MessageList.tsx +++ b/frontend/src/components/MessageList.tsx @@ -40,8 +40,11 @@ interface MessageListProps { loading: boolean; loadingOlder?: boolean; hasOlderMessages?: boolean; - unreadMarkerLastReadAt?: number | null; + /** Id of the oldest unread message, from the server. Null when nothing is unread. */ + unreadMarkerMessageId?: number | null; onDismissUnreadMarker?: () => void; + /** Called when the unread boundary is not in loaded history and must be jumped to. */ + onNavigateToUnread?: (messageId: number) => void; onSenderClick?: (sender: string) => void; onLoadOlder?: () => void; onResendChannelMessage?: (messageId: number, newTimestamp?: boolean) => void; @@ -387,8 +390,9 @@ export function MessageList({ loading, loadingOlder = false, hasOlderMessages = false, - unreadMarkerLastReadAt, + unreadMarkerMessageId, onDismissUnreadMarker, + onNavigateToUnread, onSenderClick, onLoadOlder, onResendChannelMessage, @@ -714,17 +718,32 @@ export function MessageList({ }; }, [messages, onResendChannelMessage]); + /** + * Located by message id, not by timestamp. The previous `received_at > boundary` + * scan returned 0 — the top of the loaded window — whenever the real boundary was + * further back than anything loaded, so the divider silently pointed at the wrong + * message. Matching on identity returns -1 in that case, which is the truth: the + * boundary is elsewhere, and `boundaryOutsideWindow` below offers to go to it. + */ const unreadMarkerIndex = useMemo(() => { - if (unreadMarkerLastReadAt === undefined) { - return -1; - } + if (unreadMarkerMessageId == null) return -1; + return sortedMessages.findIndex((msg) => msg.id === unreadMarkerMessageId); + }, [sortedMessages, unreadMarkerMessageId]); - const boundary = unreadMarkerLastReadAt ?? 0; - return sortedMessages.findIndex((msg) => !msg.outgoing && msg.received_at > boundary); - }, [sortedMessages, unreadMarkerLastReadAt]); + // Unread exists, but the message it starts at has not been loaded. + const boundaryOutsideWindow = unreadMarkerMessageId != null && unreadMarkerIndex === -1; const syncJumpToUnreadVisibility = useCallback(() => { - if (unreadMarkerIndex === -1 || jumpToUnreadDismissed) { + if (jumpToUnreadDismissed) { + setShowJumpToUnread(false); + return; + } + // Boundary is real but out of the loaded window: always offer the jump. + if (boundaryOutsideWindow) { + setShowJumpToUnread(true); + return; + } + if (unreadMarkerIndex === -1) { setShowJumpToUnread(false); return; } @@ -756,7 +775,7 @@ export function MessageList({ markerRect.right <= listRect.right; setShowJumpToUnread(!markerVisible); - }, [jumpToUnreadDismissed, unreadMarkerIndex]); + }, [jumpToUnreadDismissed, unreadMarkerIndex, boundaryOutsideWindow]); // Refs for scroll handler to read without causing callback recreation const onLoadOlderRef = useRef(onLoadOlder); @@ -1355,7 +1374,12 @@ export function MessageList({