diff --git a/frontend/src/hooks/useConversationMessages.ts b/frontend/src/hooks/useConversationMessages.ts index 9271a2a..29bcc93 100644 --- a/frontend/src/hooks/useConversationMessages.ts +++ b/frontend/src/hooks/useConversationMessages.ts @@ -21,6 +21,24 @@ interface InternalCachedConversationEntry extends CachedConversationEntry { export class ConversationMessageCache { private readonly cache = new Map(); + private normalizeEntry(entry: CachedConversationEntry): InternalCachedConversationEntry { + let messages = entry.messages; + let hasOlderMessages = entry.hasOlderMessages; + + if (messages.length > MAX_MESSAGES_PER_ENTRY) { + messages = [...messages] + .sort((a, b) => b.received_at - a.received_at) + .slice(0, MAX_MESSAGES_PER_ENTRY); + hasOlderMessages = true; + } + + return { + messages, + hasOlderMessages, + contentKeys: new Set(messages.map((message) => getMessageContentKey(message))), + }; + } + get(id: string): CachedConversationEntry | undefined { const entry = this.cache.get(id); if (!entry) return undefined; @@ -33,17 +51,7 @@ export class ConversationMessageCache { } set(id: string, entry: CachedConversationEntry): void { - const contentKeys = new Set(entry.messages.map((message) => getMessageContentKey(message))); - if (entry.messages.length > MAX_MESSAGES_PER_ENTRY) { - const trimmed = [...entry.messages] - .sort((a, b) => b.received_at - a.received_at) - .slice(0, MAX_MESSAGES_PER_ENTRY); - entry = { ...entry, messages: trimmed, hasOlderMessages: true }; - } - const internalEntry: InternalCachedConversationEntry = { - ...entry, - contentKeys, - }; + const internalEntry = this.normalizeEntry(entry); this.cache.delete(id); this.cache.set(id, internalEntry); if (this.cache.size > MAX_CACHED_CONVERSATIONS) { @@ -69,15 +77,12 @@ export class ConversationMessageCache { } if (entry.contentKeys.has(contentKey)) return false; if (entry.messages.some((message) => message.id === msg.id)) return false; - entry.contentKeys.add(contentKey); - entry.messages = [...entry.messages, msg]; - if (entry.messages.length > MAX_MESSAGES_PER_ENTRY) { - entry.messages = [...entry.messages] - .sort((a, b) => b.received_at - a.received_at) - .slice(0, MAX_MESSAGES_PER_ENTRY); - } + const nextEntry = this.normalizeEntry({ + messages: [...entry.messages, msg], + hasOlderMessages: entry.hasOlderMessages, + }); this.cache.delete(id); - this.cache.set(id, entry); + this.cache.set(id, nextEntry); return true; } @@ -123,11 +128,13 @@ export class ConversationMessageCache { } this.cache.delete(oldId); - this.cache.set(newId, { - messages: mergedMessages, - hasOlderMessages: newEntry.hasOlderMessages || oldEntry.hasOlderMessages, - contentKeys: new Set([...newEntry.contentKeys, ...oldEntry.contentKeys]), - }); + this.cache.set( + newId, + this.normalizeEntry({ + messages: mergedMessages, + hasOlderMessages: newEntry.hasOlderMessages || oldEntry.hasOlderMessages, + }) + ); } clear(): void { diff --git a/frontend/src/test/integration.test.ts b/frontend/src/test/integration.test.ts index e29796b..d28d05a 100644 --- a/frontend/src/test/integration.test.ts +++ b/frontend/src/test/integration.test.ts @@ -167,17 +167,15 @@ describe('Integration: Duplicate Message Handling', () => { }); }); -describe('Integration: No phantom unreads from mesh echoes (hitlist #8 regression)', () => { - it('does not increment unread when a mesh echo arrives after many unique messages', () => { +describe('Integration: Trimmed cache entries can reappear (hitlist #7 regression)', () => { + it('increments unread when an evicted inactive-conversation message arrives again', () => { const state = createMockState(); const convKey = 'channel_busy'; - // Deliver 1001 unique messages — exceeding the old global - // seenMessageContentRef prune threshold (1000→500). Under the old - // 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 - // Cached messages remain the source of truth for inactive-conversation dedup. + // Deliver enough unique messages to evict msg-0 from the inactive + // conversation cache. Once it falls out of that window, a later arrival + // with the same content should be allowed back in instead of being + // suppressed forever by a stale content key. const MESSAGE_COUNT = 1001; for (let i = 0; i < MESSAGE_COUNT; i++) { const msg: Message = { @@ -219,9 +217,8 @@ describe('Integration: No phantom unreads from mesh echoes (hitlist #8 regressio }; const result = handleMessageEvent(state, echo, 'other_active_conv'); - // Must NOT increment unread — the echo is a duplicate - expect(result.unreadIncremented).toBe(false); - expect(state.unreadCounts[stateKey]).toBe(MESSAGE_COUNT); + expect(result.unreadIncremented).toBe(true); + expect(state.unreadCounts[stateKey]).toBe(MESSAGE_COUNT + 1); }); }); diff --git a/frontend/src/test/messageCache.test.ts b/frontend/src/test/messageCache.test.ts index 2f857dc..3d9dcd4 100644 --- a/frontend/src/test/messageCache.test.ts +++ b/frontend/src/test/messageCache.test.ts @@ -214,6 +214,76 @@ describe('messageCache', () => { expect(entry!.messages.some((m) => m.id === 0)).toBe(false); }); + it('allows a trimmed-out message to be re-added after set() trimming', () => { + const messages = Array.from({ length: MAX_MESSAGES_PER_ENTRY + 1 }, (_, i) => + createMessage({ + id: i, + text: `message-${i}`, + received_at: 1700000000 + i, + sender_timestamp: 1700000000 + i, + }) + ); + + messageCache.set('conv1', createEntry(messages)); + + const trimmedOut = createMessage({ + id: 10_000, + text: 'message-0', + received_at: 1800000000, + sender_timestamp: 1700000000, + }); + + expect(messageCache.addMessage('conv1', trimmedOut)).toBe(true); + const entry = messageCache.get('conv1'); + expect(entry!.messages.some((m) => m.id === 10_000)).toBe(true); + }); + + it('allows a trimmed-out message to be re-added after addMessage() trimming', () => { + const messages = Array.from({ length: MAX_MESSAGES_PER_ENTRY - 1 }, (_, i) => + createMessage({ + id: i, + text: `message-${i}`, + received_at: 1700000000 + i, + sender_timestamp: 1700000000 + i, + }) + ); + messageCache.set('conv1', createEntry(messages)); + + expect( + messageCache.addMessage( + 'conv1', + createMessage({ + id: MAX_MESSAGES_PER_ENTRY, + text: 'newest-a', + received_at: 1800000000, + sender_timestamp: 1800000000, + }) + ) + ).toBe(true); + expect( + messageCache.addMessage( + 'conv1', + createMessage({ + id: MAX_MESSAGES_PER_ENTRY + 1, + text: 'newest-b', + received_at: 1800000001, + sender_timestamp: 1800000001, + }) + ) + ).toBe(true); + + const readdedTrimmedMessage = createMessage({ + id: 10_001, + text: 'message-0', + received_at: 1900000000, + sender_timestamp: 1700000000, + }); + + expect(messageCache.addMessage('conv1', readdedTrimmedMessage)).toBe(true); + const entry = messageCache.get('conv1'); + expect(entry!.messages.some((m) => m.id === 10_001)).toBe(true); + }); + 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);