Be cleaner about message cache dedupe after trimming inactive convos

This commit is contained in:
Jack Kingsman
2026-03-19 21:03:20 -07:00
parent 8ae600d010
commit cf314e02ff
3 changed files with 109 additions and 35 deletions
+31 -24
View File
@@ -21,6 +21,24 @@ interface InternalCachedConversationEntry extends CachedConversationEntry {
export class ConversationMessageCache { export class ConversationMessageCache {
private readonly cache = new Map<string, InternalCachedConversationEntry>(); private readonly cache = new Map<string, InternalCachedConversationEntry>();
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 { get(id: string): CachedConversationEntry | undefined {
const entry = this.cache.get(id); const entry = this.cache.get(id);
if (!entry) return undefined; if (!entry) return undefined;
@@ -33,17 +51,7 @@ export class ConversationMessageCache {
} }
set(id: string, entry: CachedConversationEntry): void { set(id: string, entry: CachedConversationEntry): void {
const contentKeys = new Set(entry.messages.map((message) => getMessageContentKey(message))); const internalEntry = this.normalizeEntry(entry);
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,
};
this.cache.delete(id); this.cache.delete(id);
this.cache.set(id, internalEntry); this.cache.set(id, internalEntry);
if (this.cache.size > MAX_CACHED_CONVERSATIONS) { if (this.cache.size > MAX_CACHED_CONVERSATIONS) {
@@ -69,15 +77,12 @@ export class ConversationMessageCache {
} }
if (entry.contentKeys.has(contentKey)) return false; if (entry.contentKeys.has(contentKey)) return false;
if (entry.messages.some((message) => message.id === msg.id)) return false; if (entry.messages.some((message) => message.id === msg.id)) return false;
entry.contentKeys.add(contentKey); const nextEntry = this.normalizeEntry({
entry.messages = [...entry.messages, msg]; messages: [...entry.messages, msg],
if (entry.messages.length > MAX_MESSAGES_PER_ENTRY) { hasOlderMessages: entry.hasOlderMessages,
entry.messages = [...entry.messages] });
.sort((a, b) => b.received_at - a.received_at)
.slice(0, MAX_MESSAGES_PER_ENTRY);
}
this.cache.delete(id); this.cache.delete(id);
this.cache.set(id, entry); this.cache.set(id, nextEntry);
return true; return true;
} }
@@ -123,11 +128,13 @@ export class ConversationMessageCache {
} }
this.cache.delete(oldId); this.cache.delete(oldId);
this.cache.set(newId, { this.cache.set(
messages: mergedMessages, newId,
hasOlderMessages: newEntry.hasOlderMessages || oldEntry.hasOlderMessages, this.normalizeEntry({
contentKeys: new Set([...newEntry.contentKeys, ...oldEntry.contentKeys]), messages: mergedMessages,
}); hasOlderMessages: newEntry.hasOlderMessages || oldEntry.hasOlderMessages,
})
);
} }
clear(): void { clear(): void {
+8 -11
View File
@@ -167,17 +167,15 @@ describe('Integration: Duplicate Message Handling', () => {
}); });
}); });
describe('Integration: No phantom unreads from mesh echoes (hitlist #8 regression)', () => { describe('Integration: Trimmed cache entries can reappear (hitlist #7 regression)', () => {
it('does not increment unread when a mesh echo arrives after many unique messages', () => { it('increments unread when an evicted inactive-conversation message arrives again', () => {
const state = createMockState(); const state = createMockState();
const convKey = 'channel_busy'; const convKey = 'channel_busy';
// Deliver 1001 unique messages — exceeding the old global // Deliver enough unique messages to evict msg-0 from the inactive
// seenMessageContentRef prune threshold (1000→500). Under the old // conversation cache. Once it falls out of that window, a later arrival
// dual-set design the global set would drop msg-0's key during pruning, // with the same content should be allowed back in instead of being
// so a later mesh echo of msg-0 would pass the global check and // suppressed forever by a stale content key.
// phantom-increment unread. With the fix, messageCache's per-conversation
// Cached messages remain the source of truth for inactive-conversation dedup.
const MESSAGE_COUNT = 1001; const MESSAGE_COUNT = 1001;
for (let i = 0; i < MESSAGE_COUNT; i++) { for (let i = 0; i < MESSAGE_COUNT; i++) {
const msg: Message = { 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'); const result = handleMessageEvent(state, echo, 'other_active_conv');
// Must NOT increment unread — the echo is a duplicate expect(result.unreadIncremented).toBe(true);
expect(result.unreadIncremented).toBe(false); expect(state.unreadCounts[stateKey]).toBe(MESSAGE_COUNT + 1);
expect(state.unreadCounts[stateKey]).toBe(MESSAGE_COUNT);
}); });
}); });
+70
View File
@@ -214,6 +214,76 @@ describe('messageCache', () => {
expect(entry!.messages.some((m) => m.id === 0)).toBe(false); 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', () => { it('auto-creates a minimal entry for never-visited conversations and returns true', () => {
const msg = createMessage({ id: 10, text: 'First contact' }); const msg = createMessage({ id: 10, text: 'First contact' });
const result = messageCache.addMessage('new_conv', msg); const result = messageCache.addMessage('new_conv', msg);