diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 685e56c..ac1a7c0 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -234,8 +234,7 @@ export function App() { fetchNewerMessages, jumpToBottom, reloadCurrentConversation, - addMessageIfNew, - receiveRealtimeMessage, + observeMessage, receiveMessageAck, reconcileOnReconnect, renameConversationMessages, @@ -249,10 +248,9 @@ export function App() { mentions, lastMessageTimes, unreadLastReadAts, - incrementUnread, + recordMessageEvent, renameConversationState, markAllRead, - trackNewMessage, refreshUnreads, } = useUnreadCounts(channels, contacts, activeConversation); @@ -322,9 +320,8 @@ export function App() { blockedKeysRef, blockedNamesRef, activeConversationRef, - receiveRealtimeMessage, - trackNewMessage, - incrementUnread, + observeMessage, + recordMessageEvent, renameConversationState, checkMention, pendingDeleteFallbackRef, @@ -368,7 +365,7 @@ export function App() { activeConversationRef, setContacts, setChannels, - addMessageIfNew, + observeMessage, messageInputRef, }); const handleCreateCrackedChannel = useCallback( diff --git a/frontend/src/components/MessageList.tsx b/frontend/src/components/MessageList.tsx index 3b5ddca..092547c 100644 --- a/frontend/src/components/MessageList.tsx +++ b/frontend/src/components/MessageList.tsx @@ -329,7 +329,7 @@ export function MessageList({ }, [messages, onResendChannelMessage]); // Sort messages by received_at ascending (oldest first) - // Note: Deduplication is handled by useConversationMessages.addMessageIfNew() + // Note: Deduplication is handled by useConversationMessages.observeMessage() // and the database UNIQUE constraint on (type, conversation_key, text, sender_timestamp) const sortedMessages = useMemo( () => [...messages].sort((a, b) => a.received_at - b.received_at || a.id - b.id), diff --git a/frontend/src/hooks/index.ts b/frontend/src/hooks/index.ts index 571e07f..184c9c2 100644 --- a/frontend/src/hooks/index.ts +++ b/frontend/src/hooks/index.ts @@ -1,5 +1,5 @@ export { useUnreadCounts } from './useUnreadCounts'; -export { useConversationMessages, getMessageContentKey } from './useConversationMessages'; +export { useConversationMessages } from './useConversationMessages'; export { useRadioControl } from './useRadioControl'; export { useRepeaterDashboard } from './useRepeaterDashboard'; export { useAppShell } from './useAppShell'; diff --git a/frontend/src/hooks/useConversationActions.ts b/frontend/src/hooks/useConversationActions.ts index f60b50d..476e9d4 100644 --- a/frontend/src/hooks/useConversationActions.ts +++ b/frontend/src/hooks/useConversationActions.ts @@ -10,7 +10,7 @@ interface UseConversationActionsArgs { activeConversationRef: MutableRefObject; setContacts: React.Dispatch>; setChannels: React.Dispatch>; - addMessageIfNew: (msg: Message) => boolean; + observeMessage: (msg: Message) => { added: boolean; activeConversation: boolean }; messageInputRef: RefObject; } @@ -31,7 +31,7 @@ export function useConversationActions({ activeConversationRef, setContacts, setChannels, - addMessageIfNew, + observeMessage, messageInputRef, }: UseConversationActionsArgs): UseConversationActionsResult { const mergeChannelIntoList = useCallback( @@ -60,10 +60,10 @@ export function useConversationActions({ : await api.sendDirectMessage(activeConversation.id, text); if (activeConversationRef.current?.id === conversationId) { - addMessageIfNew(sent); + observeMessage(sent); } }, - [activeConversation, activeConversationRef, addMessageIfNew] + [activeConversation, activeConversationRef, observeMessage] ); const handleResendChannelMessage = useCallback( @@ -77,7 +77,7 @@ export function useConversationActions({ activeConversationRef.current?.type === 'channel' && activeConversationRef.current.id === resentMessage.conversation_key ) { - addMessageIfNew(resentMessage); + observeMessage(resentMessage); } toast.success(newTimestamp ? 'Message resent with new timestamp' : 'Message resent'); } catch (err) { @@ -86,7 +86,7 @@ export function useConversationActions({ }); } }, - [activeConversationRef, addMessageIfNew] + [activeConversationRef, observeMessage] ); const handleSetChannelFloodScopeOverride = useCallback( diff --git a/frontend/src/hooks/useConversationMessages.ts b/frontend/src/hooks/useConversationMessages.ts index 21c2495..2e752b4 100644 --- a/frontend/src/hooks/useConversationMessages.ts +++ b/frontend/src/hooks/useConversationMessages.ts @@ -1,16 +1,9 @@ -import { - useCallback, - useEffect, - useRef, - useState, - type Dispatch, - type MutableRefObject, - type SetStateAction, -} from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { toast } from '../components/ui/sonner'; import { api, isAbortError } from '../api'; import * as messageCache from '../messageCache'; import type { Conversation, Message, MessagePath } from '../types'; +import { getMessageContentKey } from '../utils/messageIdentity'; const MAX_PENDING_ACKS = 500; const MESSAGE_PAGE_SIZE = 200; @@ -56,15 +49,6 @@ export function mergePendingAck( return existing; } -// Generate a key for deduplicating messages by content -export function getMessageContentKey(msg: Message): string { - // When sender_timestamp exists, dedup by content (catches radio-path duplicates with different IDs). - // When null, include msg.id so each message gets a unique key — avoids silently dropping - // different messages that share the same text and received_at second. - const ts = msg.sender_timestamp ?? `r${msg.received_at}-${msg.id}`; - return `${msg.type}-${msg.conversation_key}-${msg.text}-${ts}`; -} - interface UseConversationMessagesResult { messages: Message[]; messagesLoading: boolean; @@ -72,16 +56,11 @@ interface UseConversationMessagesResult { hasOlderMessages: boolean; hasNewerMessages: boolean; loadingNewer: boolean; - hasNewerMessagesRef: MutableRefObject; - setMessages: Dispatch>; fetchOlderMessages: () => Promise; fetchNewerMessages: () => Promise; jumpToBottom: () => void; reloadCurrentConversation: () => void; - addMessageIfNew: (msg: Message) => boolean; - updateMessageAck: (messageId: number, ackCount: number, paths?: MessagePath[]) => void; - triggerReconcile: () => void; - receiveRealtimeMessage: (msg: Message) => { added: boolean; activeConversation: boolean }; + observeMessage: (msg: Message) => { added: boolean; activeConversation: boolean }; receiveMessageAck: (messageId: number, ackCount: number, paths?: MessagePath[]) => void; reconcileOnReconnect: () => void; renameConversationMessages: (oldId: string, newId: string) => void; @@ -469,14 +448,6 @@ export function useConversationMessages( setReloadVersion((current) => current + 1); }, [activeConversation]); - const triggerReconcile = useCallback(() => { - if (!isMessageConversation(activeConversation)) return; - const controller = new AbortController(); - const requestId = latestReconcileRequestIdRef.current + 1; - latestReconcileRequestIdRef.current = requestId; - reconcileFromBackend(activeConversation, controller.signal, requestId); - }, [activeConversation, reconcileFromBackend]); - const reconcileOnReconnect = useCallback(() => { if (!isMessageConversation(activeConversation)) { return; @@ -537,7 +508,6 @@ export function useConversationMessages( ) { messageCache.set(prevId, { messages: messagesRef.current, - seenContent: new Set(seenMessageContent.current), hasOlderMessages: hasOlderMessagesRef.current, }); } @@ -582,7 +552,9 @@ export function useConversationMessages( const cached = messageCache.get(activeConversation.id); if (cached) { setMessages(cached.messages); - seenMessageContent.current = new Set(cached.seenContent); + seenMessageContent.current = new Set( + cached.messages.map((message) => getMessageContentKey(message)) + ); setHasOlderMessages(cached.hasOlderMessages); setMessagesLoading(false); const requestId = latestReconcileRequestIdRef.current + 1; @@ -599,9 +571,8 @@ export function useConversationMessages( // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeConversation?.id, activeConversation?.type, targetMessageId, reloadVersion]); - // Add a message if it's new (deduplication) - // Returns true if the message was added, false if it was a duplicate - const addMessageIfNew = useCallback( + // Add a message to the active conversation if it is new. + const appendActiveMessageIfNew = useCallback( (msg: Message): boolean => { const msgWithPendingAck = applyPendingAck(msg); const contentKey = getMessageContentKey(msgWithPendingAck); @@ -679,7 +650,7 @@ export function useConversationMessages( [updateMessageAck] ); - const receiveRealtimeMessage = useCallback( + const observeMessage = useCallback( (msg: Message): { added: boolean; activeConversation: boolean } => { const msgWithPendingAck = applyPendingAck(msg); const activeConversationMessage = isActiveConversationMessage( @@ -693,22 +664,17 @@ export function useConversationMessages( } return { - added: addMessageIfNew(msgWithPendingAck), + added: appendActiveMessageIfNew(msgWithPendingAck), activeConversation: true, }; } - const contentKey = getMessageContentKey(msgWithPendingAck); return { - added: messageCache.addMessage( - msgWithPendingAck.conversation_key, - msgWithPendingAck, - contentKey - ), + added: messageCache.addMessage(msgWithPendingAck.conversation_key, msgWithPendingAck), activeConversation: false, }; }, - [activeConversation, addMessageIfNew, applyPendingAck, hasNewerMessagesRef] + [activeConversation, appendActiveMessageIfNew, applyPendingAck, hasNewerMessagesRef] ); const renameConversationMessages = useCallback((oldId: string, newId: string) => { @@ -730,16 +696,11 @@ export function useConversationMessages( hasOlderMessages, hasNewerMessages, loadingNewer, - hasNewerMessagesRef, - setMessages, fetchOlderMessages, fetchNewerMessages, jumpToBottom, reloadCurrentConversation, - addMessageIfNew, - updateMessageAck, - triggerReconcile, - receiveRealtimeMessage, + observeMessage, receiveMessageAck, reconcileOnReconnect, renameConversationMessages, diff --git a/frontend/src/hooks/useRealtimeAppState.ts b/frontend/src/hooks/useRealtimeAppState.ts index 3e25f6a..8c86c30 100644 --- a/frontend/src/hooks/useRealtimeAppState.ts +++ b/frontend/src/hooks/useRealtimeAppState.ts @@ -35,9 +35,13 @@ interface UseRealtimeAppStateArgs { blockedKeysRef: MutableRefObject; blockedNamesRef: MutableRefObject; activeConversationRef: MutableRefObject; - receiveRealtimeMessage: (msg: Message) => { added: boolean; activeConversation: boolean }; - trackNewMessage: (msg: Message) => void; - incrementUnread: (stateKey: string, hasMention?: boolean) => void; + observeMessage: (msg: Message) => { added: boolean; activeConversation: boolean }; + recordMessageEvent: (args: { + msg: Message; + activeConversation: boolean; + isNewMessage: boolean; + hasMention?: boolean; + }) => void; renameConversationState: (oldStateKey: string, newStateKey: string) => void; checkMention: (text: string) => boolean; pendingDeleteFallbackRef: MutableRefObject; @@ -83,9 +87,8 @@ export function useRealtimeAppState({ blockedKeysRef, blockedNamesRef, activeConversationRef, - receiveRealtimeMessage, - trackNewMessage, - incrementUnread, + observeMessage, + recordMessageEvent, renameConversationState, checkMention, pendingDeleteFallbackRef, @@ -179,23 +182,13 @@ export function useRealtimeAppState({ } const { added: isNewMessage, activeConversation: isForActiveConversation } = - receiveRealtimeMessage(msg); - - trackNewMessage(msg); - - if (!isForActiveConversation) { - if (!msg.outgoing && isNewMessage) { - let stateKey: string | null = null; - if (msg.type === 'CHAN' && msg.conversation_key) { - stateKey = getStateKey('channel', msg.conversation_key); - } else if (msg.type === 'PRIV' && msg.conversation_key) { - stateKey = getStateKey('contact', msg.conversation_key); - } - if (stateKey) { - incrementUnread(stateKey, checkMention(msg.text)); - } - } - } + observeMessage(msg); + recordMessageEvent({ + msg, + activeConversation: isForActiveConversation, + isNewMessage, + hasMention: checkMention(msg.text), + }); if (!msg.outgoing && isNewMessage) { notifyIncomingMessage?.(msg); @@ -261,15 +254,15 @@ export function useRealtimeAppState({ checkMention, fetchAllContacts, fetchConfig, - incrementUnread, renameConversationState, renameConversationMessages, maxRawPackets, mergeChannelIntoList, pendingDeleteFallbackRef, prevHealthRef, + recordMessageEvent, receiveMessageAck, - receiveRealtimeMessage, + observeMessage, refreshUnreads, reconcileOnReconnect, removeConversationMessages, @@ -278,7 +271,6 @@ export function useRealtimeAppState({ setContacts, setHealth, setRawPackets, - trackNewMessage, notifyIncomingMessage, ] ); diff --git a/frontend/src/hooks/useUnreadCounts.ts b/frontend/src/hooks/useUnreadCounts.ts index 398ae7b..623deb4 100644 --- a/frontend/src/hooks/useUnreadCounts.ts +++ b/frontend/src/hooks/useUnreadCounts.ts @@ -16,10 +16,14 @@ interface UseUnreadCountsResult { mentions: Record; lastMessageTimes: ConversationTimes; unreadLastReadAts: Record; - incrementUnread: (stateKey: string, hasMention?: boolean) => void; + recordMessageEvent: (args: { + msg: Message; + activeConversation: boolean; + isNewMessage: boolean; + hasMention?: boolean; + }) => void; renameConversationState: (oldStateKey: string, newStateKey: string) => void; markAllRead: () => void; - trackNewMessage: (msg: Message) => void; refreshUnreads: () => Promise; } @@ -162,7 +166,6 @@ export function useUnreadCounts( } }, [activeConversation]); - // Increment unread count for a conversation const incrementUnread = useCallback((stateKey: string, hasMention?: boolean) => { setUnreadCounts((prev) => ({ ...prev, @@ -176,6 +179,40 @@ export function useUnreadCounts( } }, []); + const recordMessageEvent = useCallback( + ({ + msg, + activeConversation: isActiveConversation, + isNewMessage, + hasMention, + }: { + msg: Message; + activeConversation: boolean; + isNewMessage: boolean; + hasMention?: boolean; + }) => { + let stateKey: string | null = null; + if (msg.type === 'CHAN' && msg.conversation_key) { + stateKey = getStateKey('channel', msg.conversation_key); + } else if (msg.type === 'PRIV' && msg.conversation_key) { + stateKey = getStateKey('contact', msg.conversation_key); + } + + if (!stateKey) { + return; + } + + const timestamp = msg.received_at || Math.floor(Date.now() / 1000); + const updated = setLastMessageTime(stateKey, timestamp); + setLastMessageTimes(updated); + + if (!isActiveConversation && !msg.outgoing && isNewMessage) { + incrementUnread(stateKey, hasMention); + } + }, + [incrementUnread] + ); + const renameConversationState = useCallback((oldStateKey: string, newStateKey: string) => { if (oldStateKey === newStateKey) return; @@ -212,31 +249,14 @@ export function useUnreadCounts( }); }, []); - // Track a new incoming message for unread counts - const trackNewMessage = useCallback((msg: Message) => { - let conversationKey: string | null = null; - if (msg.type === 'CHAN' && msg.conversation_key) { - conversationKey = getStateKey('channel', msg.conversation_key); - } else if (msg.type === 'PRIV' && msg.conversation_key) { - conversationKey = getStateKey('contact', msg.conversation_key); - } - - if (conversationKey) { - const timestamp = msg.received_at || Math.floor(Date.now() / 1000); - const updated = setLastMessageTime(conversationKey, timestamp); - setLastMessageTimes(updated); - } - }, []); - return { unreadCounts, mentions, lastMessageTimes, unreadLastReadAts, - incrementUnread, + recordMessageEvent, renameConversationState, markAllRead, - trackNewMessage, refreshUnreads: fetchUnreads, }; } diff --git a/frontend/src/messageCache.ts b/frontend/src/messageCache.ts index be380f6..bfdd20b 100644 --- a/frontend/src/messageCache.ts +++ b/frontend/src/messageCache.ts @@ -8,17 +8,21 @@ */ import type { Message, MessagePath } from './types'; +import { getMessageContentKey } from './utils/messageIdentity'; export const MAX_CACHED_CONVERSATIONS = 20; export const MAX_MESSAGES_PER_ENTRY = 200; interface CacheEntry { messages: Message[]; - seenContent: Set; hasOlderMessages: boolean; } -const cache = new Map(); +interface InternalCacheEntry extends CacheEntry { + contentKeys: Set; +} + +const cache = new Map(); /** Get a cached entry and promote it to most-recently-used. */ export function get(id: string): CacheEntry | undefined { @@ -27,11 +31,15 @@ export function get(id: string): CacheEntry | undefined { // Promote to MRU: delete and re-insert cache.delete(id); cache.set(id, entry); - return entry; + return { + messages: entry.messages, + hasOlderMessages: entry.hasOlderMessages, + }; } /** Insert or update an entry at MRU position, evicting LRU if over capacity. */ export function set(id: string, entry: CacheEntry): void { + const contentKeys = new Set(entry.messages.map((message) => getMessageContentKey(message))); // Trim to most recent messages to bound memory if (entry.messages.length > MAX_MESSAGES_PER_ENTRY) { const trimmed = [...entry.messages] @@ -39,9 +47,13 @@ export function set(id: string, entry: CacheEntry): void { .slice(0, MAX_MESSAGES_PER_ENTRY); entry = { ...entry, messages: trimmed, hasOlderMessages: true }; } + const internalEntry: InternalCacheEntry = { + ...entry, + contentKeys, + }; // Remove first so re-insert moves to end cache.delete(id); - cache.set(id, entry); + cache.set(id, internalEntry); // Evict LRU (first entry) if over capacity if (cache.size > MAX_CACHED_CONVERSATIONS) { const lruKey = cache.keys().next().value as string; @@ -50,14 +62,15 @@ export function set(id: string, entry: CacheEntry): void { } /** Add a message to a cached conversation with dedup. Returns true if new, false if duplicate. */ -export function addMessage(id: string, msg: Message, contentKey: string): boolean { +export function addMessage(id: string, msg: Message): boolean { const entry = cache.get(id); + const contentKey = getMessageContentKey(msg); if (!entry) { // Auto-create a minimal entry for never-visited conversations cache.set(id, { messages: [msg], - seenContent: new Set([contentKey]), hasOlderMessages: true, + contentKeys: new Set([contentKey]), }); // Evict LRU if over capacity if (cache.size > MAX_CACHED_CONVERSATIONS) { @@ -66,9 +79,9 @@ export function addMessage(id: string, msg: Message, contentKey: string): boolea } return true; } - if (entry.seenContent.has(contentKey)) return false; + if (entry.contentKeys.has(contentKey)) return false; if (entry.messages.some((m) => m.id === msg.id)) return false; - entry.seenContent.add(contentKey); + entry.contentKeys.add(contentKey); entry.messages = [...entry.messages, msg]; // Trim if over limit (drop oldest by received_at) if (entry.messages.length > MAX_MESSAGES_PER_ENTRY) { @@ -163,8 +176,8 @@ export function rename(oldId: string, newId: string): void { cache.delete(oldId); cache.set(newId, { messages: mergedMessages, - seenContent: new Set([...newEntry.seenContent, ...oldEntry.seenContent]), hasOlderMessages: newEntry.hasOlderMessages || oldEntry.hasOlderMessages, + contentKeys: new Set([...newEntry.contentKeys, ...oldEntry.contentKeys]), }); } diff --git a/frontend/src/test/appFavorites.test.tsx b/frontend/src/test/appFavorites.test.tsx index b6564d4..97af6e8 100644 --- a/frontend/src/test/appFavorites.test.tsx +++ b/frontend/src/test/appFavorites.test.tsx @@ -31,19 +31,15 @@ const mocks = vi.hoisted(() => ({ error: vi.fn(), }, hookFns: { - setMessages: vi.fn(), - fetchMessages: vi.fn(async () => {}), fetchOlderMessages: vi.fn(async () => {}), - addMessageIfNew: vi.fn(), - receiveRealtimeMessage: vi.fn(() => ({ added: false, activeConversation: false })), + observeMessage: vi.fn(() => ({ added: false, activeConversation: false })), receiveMessageAck: vi.fn(), reconcileOnReconnect: vi.fn(), renameConversationMessages: vi.fn(), removeConversationMessages: vi.fn(), clearConversationMessages: vi.fn(), - incrementUnread: vi.fn(), + recordMessageEvent: vi.fn(), markAllRead: vi.fn(), - trackNewMessage: vi.fn(), refreshUnreads: vi.fn(async () => {}), }, })); @@ -67,15 +63,11 @@ vi.mock('../hooks', async (importOriginal) => { hasOlderMessages: false, hasNewerMessages: false, loadingNewer: false, - hasNewerMessagesRef: { current: false }, - setMessages: mocks.hookFns.setMessages, - fetchMessages: mocks.hookFns.fetchMessages, fetchOlderMessages: mocks.hookFns.fetchOlderMessages, fetchNewerMessages: vi.fn(async () => {}), jumpToBottom: vi.fn(), reloadCurrentConversation: vi.fn(), - addMessageIfNew: mocks.hookFns.addMessageIfNew, - receiveRealtimeMessage: mocks.hookFns.receiveRealtimeMessage, + observeMessage: mocks.hookFns.observeMessage, receiveMessageAck: mocks.hookFns.receiveMessageAck, reconcileOnReconnect: mocks.hookFns.reconcileOnReconnect, renameConversationMessages: mocks.hookFns.renameConversationMessages, @@ -87,10 +79,9 @@ vi.mock('../hooks', async (importOriginal) => { mentions: {}, lastMessageTimes: {}, unreadLastReadAts: {}, - incrementUnread: mocks.hookFns.incrementUnread, + recordMessageEvent: mocks.hookFns.recordMessageEvent, renameConversationState: vi.fn(), markAllRead: mocks.hookFns.markAllRead, - trackNewMessage: mocks.hookFns.trackNewMessage, refreshUnreads: mocks.hookFns.refreshUnreads, }), }; diff --git a/frontend/src/test/appSearchJump.test.tsx b/frontend/src/test/appSearchJump.test.tsx index b2c9c8a..f871b84 100644 --- a/frontend/src/test/appSearchJump.test.tsx +++ b/frontend/src/test/appSearchJump.test.tsx @@ -37,14 +37,11 @@ vi.mock('../hooks', async (importOriginal) => { hasOlderMessages: false, hasNewerMessages: false, loadingNewer: false, - hasNewerMessagesRef: { current: false }, - setMessages: vi.fn(), fetchOlderMessages: vi.fn(async () => {}), fetchNewerMessages: vi.fn(async () => {}), jumpToBottom: vi.fn(), reloadCurrentConversation: vi.fn(), - addMessageIfNew: vi.fn(), - receiveRealtimeMessage: vi.fn(() => ({ added: false, activeConversation: false })), + observeMessage: vi.fn(() => ({ added: false, activeConversation: false })), receiveMessageAck: vi.fn(), reconcileOnReconnect: vi.fn(), renameConversationMessages: vi.fn(), @@ -57,10 +54,9 @@ vi.mock('../hooks', async (importOriginal) => { mentions: {}, lastMessageTimes: {}, unreadLastReadAts: {}, - incrementUnread: vi.fn(), + recordMessageEvent: vi.fn(), renameConversationState: vi.fn(), markAllRead: vi.fn(), - trackNewMessage: vi.fn(), refreshUnreads: vi.fn(), }), }; diff --git a/frontend/src/test/appStartupHash.test.tsx b/frontend/src/test/appStartupHash.test.tsx index c1dac7c..0b2fbb2 100644 --- a/frontend/src/test/appStartupHash.test.tsx +++ b/frontend/src/test/appStartupHash.test.tsx @@ -32,15 +32,11 @@ vi.mock('../hooks', async (importOriginal) => { hasOlderMessages: false, hasNewerMessages: false, loadingNewer: false, - hasNewerMessagesRef: { current: false }, - setMessages: vi.fn(), - fetchMessages: vi.fn(async () => {}), fetchOlderMessages: vi.fn(async () => {}), fetchNewerMessages: vi.fn(async () => {}), jumpToBottom: vi.fn(), reloadCurrentConversation: vi.fn(), - addMessageIfNew: vi.fn(), - receiveRealtimeMessage: vi.fn(() => ({ added: false, activeConversation: false })), + observeMessage: vi.fn(() => ({ added: false, activeConversation: false })), receiveMessageAck: vi.fn(), reconcileOnReconnect: vi.fn(), renameConversationMessages: vi.fn(), @@ -52,10 +48,9 @@ vi.mock('../hooks', async (importOriginal) => { mentions: {}, lastMessageTimes: {}, unreadLastReadAts: {}, - incrementUnread: vi.fn(), + recordMessageEvent: vi.fn(), renameConversationState: vi.fn(), markAllRead: vi.fn(), - trackNewMessage: vi.fn(), refreshUnreads: vi.fn(async () => {}), }), }; diff --git a/frontend/src/test/integration.test.ts b/frontend/src/test/integration.test.ts index 5b4fea6..e3a5d93 100644 --- a/frontend/src/test/integration.test.ts +++ b/frontend/src/test/integration.test.ts @@ -10,9 +10,9 @@ import { describe, it, expect, beforeEach } from 'vitest'; import fixtures from './fixtures/websocket_events.json'; -import { getMessageContentKey } from '../hooks/useConversationMessages'; import { getStateKey } from '../utils/conversationState'; import { mergeContactIntoList } from '../utils/contactMerge'; +import { getMessageContentKey } from '../utils/messageIdentity'; import * as messageCache from '../messageCache'; import type { Contact, Message } from '../types'; @@ -68,7 +68,7 @@ function handleMessageEvent( state.lastMessageTimes[stateKey] = msg.received_at; if (!isForActiveConversation) { - const isNew = messageCache.addMessage(msg.conversation_key, msg, contentKey); + const isNew = messageCache.addMessage(msg.conversation_key, msg); if (!msg.outgoing && isNew) { state.unreadCounts[stateKey] = (state.unreadCounts[stateKey] || 0) + 1; unreadIncremented = true; @@ -180,7 +180,7 @@ describe('Integration: No phantom unreads from mesh echoes (hitlist #8 regressio // dual-set design the global set would drop msg-0's key during pruning, // so a later mesh echo of msg-0 would pass the global check and // phantom-increment unread. With the fix, messageCache's per-conversation - // seenContent is the single source of truth and is never pruned. + // Cached messages remain the source of truth for inactive-conversation dedup. const MESSAGE_COUNT = 1001; for (let i = 0; i < MESSAGE_COUNT; i++) { const msg: Message = { @@ -362,7 +362,7 @@ describe('Integration: ACK + messageCache propagation', () => { acked: 0, sender_name: null, }; - messageCache.addMessage('pk_abc', msg, 'key-100'); + messageCache.addMessage('pk_abc', msg); messageCache.updateAck(100, 1); @@ -387,7 +387,7 @@ describe('Integration: ACK + messageCache propagation', () => { acked: 1, sender_name: null, }; - messageCache.addMessage('pk_abc', msg, 'key-101'); + messageCache.addMessage('pk_abc', msg); const longerPaths = [ { path: 'aa', received_at: 1700000001 }, @@ -416,7 +416,7 @@ describe('Integration: ACK + messageCache propagation', () => { acked: 5, sender_name: null, }; - messageCache.addMessage('pk_abc', msg, 'key-102'); + messageCache.addMessage('pk_abc', msg); // Try to update with a lower ack count messageCache.updateAck(102, 3); @@ -441,7 +441,7 @@ describe('Integration: ACK + messageCache propagation', () => { acked: 0, sender_name: null, }; - messageCache.addMessage('pk_abc', msg, 'key-103'); + messageCache.addMessage('pk_abc', msg); // Update a non-existent message ID — should not throw or modify anything messageCache.updateAck(999, 1); diff --git a/frontend/src/test/messageCache.test.ts b/frontend/src/test/messageCache.test.ts index 4978f85..ef72146 100644 --- a/frontend/src/test/messageCache.test.ts +++ b/frontend/src/test/messageCache.test.ts @@ -27,11 +27,7 @@ function createMessage(overrides: Partial = {}): Message { } function createEntry(messages: Message[] = [], hasOlderMessages = false) { - const seenContent = new Set(); - for (const msg of messages) { - seenContent.add(`${msg.type}-${msg.conversation_key}-${msg.text}-${msg.sender_timestamp}`); - } - return { messages, seenContent, hasOlderMessages }; + return { messages, hasOlderMessages }; } describe('messageCache', () => { @@ -155,11 +151,7 @@ describe('messageCache', () => { messageCache.set('conv1', createEntry([])); const msg = createMessage({ id: 10, text: 'New message' }); - const result = messageCache.addMessage( - 'conv1', - msg, - 'CHAN-channel123-New message-1700000000' - ); + const result = messageCache.addMessage('conv1', msg); expect(result).toBe(true); const entry = messageCache.get('conv1'); @@ -171,12 +163,11 @@ describe('messageCache', () => { messageCache.set('conv1', createEntry([])); const msg1 = createMessage({ id: 10, text: 'Hello' }); - const contentKey = 'CHAN-channel123-Hello-1700000000'; - expect(messageCache.addMessage('conv1', msg1, contentKey)).toBe(true); + expect(messageCache.addMessage('conv1', msg1)).toBe(true); // Same content key, different message id const msg2 = createMessage({ id: 11, text: 'Hello' }); - expect(messageCache.addMessage('conv1', msg2, contentKey)).toBe(false); + expect(messageCache.addMessage('conv1', msg2)).toBe(false); const entry = messageCache.get('conv1'); expect(entry!.messages).toHaveLength(1); @@ -187,9 +178,7 @@ describe('messageCache', () => { // Same id, different content key const msg = createMessage({ id: 10, text: 'Different' }); - expect(messageCache.addMessage('conv1', msg, 'CHAN-channel123-Different-1700000000')).toBe( - false - ); + expect(messageCache.addMessage('conv1', msg)).toBe(false); const entry = messageCache.get('conv1'); expect(entry!.messages).toHaveLength(1); @@ -208,11 +197,7 @@ describe('messageCache', () => { text: 'newest', received_at: 1700000000 + MAX_MESSAGES_PER_ENTRY, }); - const result = messageCache.addMessage( - 'conv1', - newMsg, - `CHAN-channel123-newest-${newMsg.sender_timestamp}` - ); + const result = messageCache.addMessage('conv1', newMsg); expect(result).toBe(true); const entry = messageCache.get('conv1'); @@ -225,11 +210,7 @@ describe('messageCache', () => { it('auto-creates a minimal entry for never-visited conversations and returns true', () => { const msg = createMessage({ id: 10, text: 'First contact' }); - const result = messageCache.addMessage( - 'new_conv', - msg, - 'CHAN-channel123-First contact-1700000000' - ); + const result = messageCache.addMessage('new_conv', msg); expect(result).toBe(true); const entry = messageCache.get('new_conv'); @@ -237,7 +218,6 @@ describe('messageCache', () => { expect(entry!.messages).toHaveLength(1); expect(entry!.messages[0].text).toBe('First contact'); expect(entry!.hasOlderMessages).toBe(true); - expect(entry!.seenContent.has('CHAN-channel123-First contact-1700000000')).toBe(true); }); it('promotes entry to MRU on addMessage', () => { @@ -248,7 +228,7 @@ describe('messageCache', () => { // addMessage to conv0 (currently LRU) should promote it const msg = createMessage({ id: 999, text: 'Incoming WS message' }); - messageCache.addMessage('conv0', msg, 'CHAN-channel123-Incoming WS message-1700000000'); + messageCache.addMessage('conv0', msg); // Add one more — conv1 should now be LRU and get evicted, not conv0 messageCache.set('conv_new', createEntry()); @@ -259,11 +239,10 @@ describe('messageCache', () => { it('returns false for duplicate delivery to auto-created entry', () => { const msg = createMessage({ id: 10, text: 'Echo' }); - const contentKey = 'CHAN-channel123-Echo-1700000000'; - expect(messageCache.addMessage('new_conv', msg, contentKey)).toBe(true); + expect(messageCache.addMessage('new_conv', msg)).toBe(true); // Duplicate via mesh echo - expect(messageCache.addMessage('new_conv', msg, contentKey)).toBe(false); + expect(messageCache.addMessage('new_conv', msg)).toBe(false); const entry = messageCache.get('new_conv'); expect(entry!.messages).toHaveLength(1); diff --git a/frontend/src/test/useConversationActions.test.ts b/frontend/src/test/useConversationActions.test.ts index b0ebb26..1734580 100644 --- a/frontend/src/test/useConversationActions.test.ts +++ b/frontend/src/test/useConversationActions.test.ts @@ -63,7 +63,7 @@ function createArgs(overrides: Partial activeConversationRef: { current: activeConversation }, setContacts: vi.fn(), setChannels: vi.fn(), - addMessageIfNew: vi.fn(() => true), + observeMessage: vi.fn(() => ({ added: true, activeConversation: true })), messageInputRef: { current: { appendText: vi.fn() } }, ...overrides, }; @@ -85,7 +85,7 @@ describe('useConversationActions', () => { }); expect(mocks.api.sendChannelMessage).toHaveBeenCalledWith(publicChannel.key, sentMessage.text); - expect(args.addMessageIfNew).toHaveBeenCalledWith(sentMessage); + expect(args.observeMessage).toHaveBeenCalledWith(sentMessage); }); it('does not append a sent message after the active conversation changes', async () => { @@ -111,7 +111,7 @@ describe('useConversationActions', () => { await sendPromise; }); - expect(args.addMessageIfNew).not.toHaveBeenCalled(); + expect(args.observeMessage).not.toHaveBeenCalled(); }); it('appends sender mentions into the message input', () => { @@ -146,7 +146,7 @@ describe('useConversationActions', () => { }); expect(mocks.api.resendChannelMessage).toHaveBeenCalledWith(sentMessage.id, true); - expect(args.addMessageIfNew).toHaveBeenCalledWith(resentMessage); + expect(args.observeMessage).toHaveBeenCalledWith(resentMessage); }); it('does not append a byte-perfect resend locally', async () => { @@ -162,7 +162,7 @@ describe('useConversationActions', () => { await result.current.handleResendChannelMessage(sentMessage.id, false); }); - expect(args.addMessageIfNew).not.toHaveBeenCalled(); + expect(args.observeMessage).not.toHaveBeenCalled(); }); it('does not append a resend if the user has switched conversations', async () => { @@ -190,7 +190,7 @@ describe('useConversationActions', () => { await resendPromise; }); - expect(args.addMessageIfNew).not.toHaveBeenCalled(); + expect(args.observeMessage).not.toHaveBeenCalled(); }); it('merges returned contact data after path discovery', async () => { diff --git a/frontend/src/test/useConversationMessages.race.test.ts b/frontend/src/test/useConversationMessages.race.test.ts index 8d8314b..15f1b1b 100644 --- a/frontend/src/test/useConversationMessages.race.test.ts +++ b/frontend/src/test/useConversationMessages.race.test.ts @@ -76,11 +76,11 @@ describe('useConversationMessages ACK ordering', () => { const paths = [{ path: 'A1B2', received_at: 1700000010 }]; act(() => { - result.current.updateMessageAck(42, 2, paths); + result.current.receiveMessageAck(42, 2, paths); }); act(() => { - const added = result.current.addMessageIfNew( + const { added } = result.current.observeMessage( createMessage({ id: 42, acked: 0, paths: null }) ); expect(added).toBe(true); @@ -100,7 +100,7 @@ describe('useConversationMessages ACK ordering', () => { const paths = [{ path: 'C3D4', received_at: 1700000011 }]; act(() => { - result.current.updateMessageAck(42, 1, paths); + result.current.receiveMessageAck(42, 1, paths); }); deferred.resolve([createMessage({ id: 42, acked: 0, paths: null })]); @@ -118,7 +118,7 @@ describe('useConversationMessages ACK ordering', () => { await waitFor(() => expect(mockGetMessages).toHaveBeenCalledTimes(1)); act(() => { - const added = result.current.addMessageIfNew( + const { added } = result.current.observeMessage( createMessage({ id: 99, text: 'ws-arrived', @@ -153,7 +153,7 @@ describe('useConversationMessages ACK ordering', () => { await waitFor(() => expect(result.current.messagesLoading).toBe(false)); act(() => { - result.current.addMessageIfNew(createMessage({ id: 42, acked: 0, paths: null })); + result.current.observeMessage(createMessage({ id: 42, acked: 0, paths: null })); }); const highAckPaths = [ @@ -163,8 +163,8 @@ describe('useConversationMessages ACK ordering', () => { const staleAckPaths = [{ path: 'A1B2', received_at: 1700000010 }]; act(() => { - result.current.updateMessageAck(42, 3, highAckPaths); - result.current.updateMessageAck(42, 2, staleAckPaths); + result.current.receiveMessageAck(42, 3, highAckPaths); + result.current.receiveMessageAck(42, 2, staleAckPaths); }); expect(result.current.messages[0].acked).toBe(3); @@ -321,8 +321,8 @@ describe('useConversationMessages background reconcile ordering', () => { .mockReturnValueOnce(secondReconcile.promise); act(() => { - result.current.triggerReconcile(); - result.current.triggerReconcile(); + result.current.reconcileOnReconnect(); + result.current.reconcileOnReconnect(); }); secondReconcile.resolve([createMessage({ id: 42, text: 'newer snapshot', acked: 2 })]); @@ -344,9 +344,6 @@ describe('useConversationMessages background reconcile ordering', () => { messageCache.set(conv.id, { messages: [cachedMessage], - seenContent: new Set([ - `PRIV-${cachedMessage.conversation_key}-${cachedMessage.text}-${cachedMessage.sender_timestamp}`, - ]), hasOlderMessages: true, }); @@ -596,7 +593,7 @@ describe('useConversationMessages forward pagination', () => { // Simulate WS adding a message with the same content key act(() => { - result.current.addMessageIfNew( + result.current.observeMessage( createMessage({ id: 2, conversation_key: 'ch1', diff --git a/frontend/src/test/useConversationMessages.test.ts b/frontend/src/test/useConversationMessages.test.ts index 27e5143..683d1e4 100644 --- a/frontend/src/test/useConversationMessages.test.ts +++ b/frontend/src/test/useConversationMessages.test.ts @@ -5,7 +5,8 @@ */ import { describe, it, expect } from 'vitest'; -import { getMessageContentKey, mergePendingAck } from '../hooks/useConversationMessages'; +import { mergePendingAck } from '../hooks/useConversationMessages'; +import { getMessageContentKey } from '../utils/messageIdentity'; import type { Message } from '../types'; function createMessage(overrides: Partial = {}): Message { diff --git a/frontend/src/test/useRealtimeAppState.test.ts b/frontend/src/test/useRealtimeAppState.test.ts index 9f555bf..c184d98 100644 --- a/frontend/src/test/useRealtimeAppState.test.ts +++ b/frontend/src/test/useRealtimeAppState.test.ts @@ -66,9 +66,8 @@ function createRealtimeArgs(overrides: Partial ({ added: false, activeConversation: false })), - trackNewMessage: vi.fn(), - incrementUnread: vi.fn(), + observeMessage: vi.fn(() => ({ added: false, activeConversation: false })), + recordMessageEvent: vi.fn(), renameConversationState: vi.fn(), checkMention: vi.fn(() => false), pendingDeleteFallbackRef: { current: false }, @@ -181,7 +180,7 @@ describe('useRealtimeAppState', () => { it('tracks unread state for a new non-active incoming message', () => { const { args } = createRealtimeArgs({ checkMention: vi.fn(() => true), - receiveRealtimeMessage: vi.fn(() => ({ added: true, activeConversation: false })), + observeMessage: vi.fn(() => ({ added: true, activeConversation: false })), }); const { result } = renderHook(() => useRealtimeAppState(args)); @@ -190,12 +189,13 @@ describe('useRealtimeAppState', () => { result.current.onMessage?.(incomingDm); }); - expect(args.receiveRealtimeMessage).toHaveBeenCalledWith(incomingDm); - expect(args.trackNewMessage).toHaveBeenCalledWith(incomingDm); - expect(args.incrementUnread).toHaveBeenCalledWith( - `contact-${incomingDm.conversation_key}`, - true - ); + expect(args.observeMessage).toHaveBeenCalledWith(incomingDm); + expect(args.recordMessageEvent).toHaveBeenCalledWith({ + msg: incomingDm, + activeConversation: false, + isNewMessage: true, + hasMention: true, + }); expect(args.notifyIncomingMessage).toHaveBeenCalledWith(incomingDm); }); diff --git a/frontend/src/test/useUnreadCounts.test.ts b/frontend/src/test/useUnreadCounts.test.ts index 127122b..a7151fd 100644 --- a/frontend/src/test/useUnreadCounts.test.ts +++ b/frontend/src/test/useUnreadCounts.test.ts @@ -10,7 +10,8 @@ import { act, renderHook } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { useUnreadCounts } from '../hooks/useUnreadCounts'; -import type { Channel, Contact, Conversation } from '../types'; +import type { Channel, Contact, Conversation, Message } from '../types'; +import { getStateKey } from '../utils/conversationState'; // Mock api module vi.mock('../api', () => ({ @@ -57,6 +58,25 @@ function makeContact(pubkey: string): Contact { }; } +function makeMessage(overrides: Partial = {}): Message { + return { + id: 1, + type: 'PRIV', + conversation_key: CONTACT_KEY, + text: 'hello', + sender_timestamp: 1700000000, + received_at: 1700000001, + paths: null, + txt_type: 0, + signature: null, + sender_key: null, + outgoing: false, + acked: 0, + sender_name: null, + ...overrides, + }; +} + const CHANNEL_KEY = 'AABB00112233445566778899AABBCCDD'; const CONTACT_KEY = '00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff'; @@ -332,4 +352,74 @@ describe('useUnreadCounts', () => { // Raw view doesn't filter any conversation's unreads expect(result.current.unreadCounts[`channel-${CHANNEL_KEY}`]).toBe(5); }); + + it('recordMessageEvent updates last-message time and unread count for new inactive incoming messages', async () => { + const mocks = await getMockedApi(); + const { result } = renderWith({}); + + await act(async () => { + await vi.waitFor(() => expect(mocks.getUnreads).toHaveBeenCalled()); + }); + + const msg = makeMessage({ + id: 5, + type: 'CHAN', + conversation_key: CHANNEL_KEY, + received_at: 1700001234, + }); + + await act(async () => { + result.current.recordMessageEvent({ + msg, + activeConversation: false, + isNewMessage: true, + hasMention: true, + }); + }); + + expect(result.current.unreadCounts[getStateKey('channel', CHANNEL_KEY)]).toBe(1); + expect(result.current.mentions[getStateKey('channel', CHANNEL_KEY)]).toBe(true); + expect(result.current.lastMessageTimes[getStateKey('channel', CHANNEL_KEY)]).toBe(1700001234); + }); + + it('recordMessageEvent skips unread increment for active or non-new messages but still tracks time', async () => { + const mocks = await getMockedApi(); + const { result } = renderWith({}); + + await act(async () => { + await vi.waitFor(() => expect(mocks.getUnreads).toHaveBeenCalled()); + }); + + const activeMsg = makeMessage({ + id: 6, + type: 'PRIV', + conversation_key: CONTACT_KEY, + received_at: 1700002000, + }); + + await act(async () => { + result.current.recordMessageEvent({ + msg: activeMsg, + activeConversation: true, + isNewMessage: true, + hasMention: true, + }); + result.current.recordMessageEvent({ + msg: makeMessage({ + id: 7, + type: 'CHAN', + conversation_key: CHANNEL_KEY, + received_at: 1700002001, + }), + activeConversation: false, + isNewMessage: false, + hasMention: true, + }); + }); + + expect(result.current.unreadCounts[getStateKey('contact', CONTACT_KEY)]).toBeUndefined(); + expect(result.current.unreadCounts[getStateKey('channel', CHANNEL_KEY)]).toBeUndefined(); + expect(result.current.lastMessageTimes[getStateKey('contact', CONTACT_KEY)]).toBe(1700002000); + expect(result.current.lastMessageTimes[getStateKey('channel', CHANNEL_KEY)]).toBe(1700002001); + }); }); diff --git a/frontend/src/utils/messageIdentity.ts b/frontend/src/utils/messageIdentity.ts new file mode 100644 index 0000000..359bc52 --- /dev/null +++ b/frontend/src/utils/messageIdentity.ts @@ -0,0 +1,10 @@ +import type { Message } from '../types'; + +// Content identity matches the frontend's message-level dedup contract. +export function getMessageContentKey(msg: Message): string { + // When sender_timestamp exists, dedup by content (catches radio-path duplicates with different IDs). + // When null, include msg.id so each message gets a unique key — avoids silently dropping + // different messages that share the same text and received_at second. + const ts = msg.sender_timestamp ?? `r${msg.received_at}-${msg.id}`; + return `${msg.type}-${msg.conversation_key}-${msg.text}-${ts}`; +}