From 5efc3dd63fbc6186f096f9db3d36af7e4b7e9e5c Mon Sep 17 00:00:00 2001 From: Artem Rootman <4586640+artemrootman@users.noreply.github.com> Date: Sun, 5 Apr 2026 18:18:03 +0000 Subject: [PATCH] fix: pagination correctness and message enrichment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1: Poller no longer overwrites nextCursor/hasMore — those belong to the "Load older" flow. Both poller and loadOlder now dedup messages by messageId or timestamp+from fingerprint. P1: Cursor is now compound (timestamp|messageId) with stable tie-breaking sort. Messages sharing the same timestamp at page boundaries are no longer lost. P2: getMessagesPage now applies the same enrichment as getTeamData: leadSessionId propagation and slash-command-result annotation. P3: Added 3 tests for getMessagesPage covering pagination, cursor stability with same-timestamp messages, and slash command annotation. --- src/main/services/team/TeamDataService.ts | 63 +++++++++++-- .../team/messages/MessagesPanel.tsx | 25 ++--- .../services/team/TeamDataService.test.ts | 93 +++++++++++++++++++ 3 files changed, 164 insertions(+), 17 deletions(-) diff --git a/src/main/services/team/TeamDataService.ts b/src/main/services/team/TeamDataService.ts index d50390a8..4624d1be 100644 --- a/src/main/services/team/TeamDataService.ts +++ b/src/main/services/team/TeamDataService.ts @@ -782,19 +782,70 @@ export class TeamDataService { }); } - // Sort newest-first - messages.sort((a, b) => Date.parse(b.timestamp) - Date.parse(a.timestamp)); + // Enrich: propagate leadSessionId to messages missing it (same as getTeamData) + if (config.leadSessionId || messages.some((m) => m.leadSessionId)) { + messages.sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)); + const anchors: { time: number; sessionId: string }[] = []; + for (const msg of messages) { + if (msg.leadSessionId) { + anchors.push({ time: Date.parse(msg.timestamp), sessionId: msg.leadSessionId }); + } + } + if (anchors.length > 0) { + for (const msg of messages) { + if (msg.leadSessionId) continue; + const msgTime = Date.parse(msg.timestamp); + let best = anchors[0]; + let bestDist = Math.abs(msgTime - best.time); + for (const a of anchors) { + const dist = Math.abs(msgTime - a.time); + if (dist < bestDist) { + bestDist = dist; + best = a; + } else if (dist > bestDist && a.time > msgTime) { + break; + } + } + msg.leadSessionId = best.sessionId; + } + } else if (config.leadSessionId) { + for (const msg of messages) { + msg.leadSessionId = config.leadSessionId; + } + } + } - // Apply cursor filter + // Enrich: annotate slash command responses + this.annotateSlashCommandResponses(messages); + + // Sort newest-first, with stable tie-breaker by messageId + messages.sort((a, b) => { + const diff = Date.parse(b.timestamp) - Date.parse(a.timestamp); + if (diff !== 0) return diff; + return (a.messageId ?? '').localeCompare(b.messageId ?? ''); + }); + + // Apply cursor filter. Cursor format: "timestamp|messageId" (compound) + // to handle multiple messages sharing the same timestamp. if (options.beforeTimestamp) { - const cursorMs = Date.parse(options.beforeTimestamp); - messages = messages.filter((m) => Date.parse(m.timestamp) < cursorMs); + const [cursorTs, cursorId] = options.beforeTimestamp.split('|'); + const cursorMs = Date.parse(cursorTs); + messages = messages.filter((m) => { + const ms = Date.parse(m.timestamp); + if (ms < cursorMs) return true; + if (ms > cursorMs) return false; + // Same timestamp — use messageId tie-breaker + if (!cursorId) return false; + return (m.messageId ?? '').localeCompare(cursorId) > 0; + }); } // Paginate const hasMore = messages.length > options.limit; const page = messages.slice(0, options.limit); - const nextCursor = hasMore && page.length > 0 ? page[page.length - 1].timestamp : null; + const lastMsg = page[page.length - 1]; + const nextCursor = + hasMore && lastMsg ? `${lastMsg.timestamp}|${lastMsg.messageId ?? ''}` : null; return { messages: page, nextCursor, hasMore }; } diff --git a/src/renderer/components/team/messages/MessagesPanel.tsx b/src/renderer/components/team/messages/MessagesPanel.tsx index 7ce5a006..9294d9b5 100644 --- a/src/renderer/components/team/messages/MessagesPanel.tsx +++ b/src/renderer/components/team/messages/MessagesPanel.tsx @@ -159,24 +159,20 @@ export const MessagesPanel = memo(function MessagesPanel({ })(); }, [teamName]); // eslint-disable-line react-hooks/exhaustive-deps -- intentionally only on teamName change - // Auto-refresh: poll for new messages (newest page only) + // Auto-refresh: poll for NEW messages only (prepend to head). + // Does NOT touch nextCursor/hasMore — those belong to the "Load older" flow. useEffect(() => { if (!isTeamAlive && leadActivity !== 'active') return; const interval = setInterval(async () => { try { const page = await api.teams.getMessagesPage(teamName, { limit: PAGE_SIZE }); setFetchedMessages((prev) => { - // Merge: keep older messages that aren't in the new page - const newIds = new Set(page.messages.map((m) => m.messageId ?? m.timestamp)); - const older = prev.filter( - (m) => - !newIds.has(m.messageId ?? m.timestamp) && - !page.messages.some((n) => n.timestamp === m.timestamp && n.from === m.from) + const existingIds = new Set(prev.map((m) => m.messageId ?? `${m.timestamp}\0${m.from}`)); + const newMessages = page.messages.filter( + (m) => !existingIds.has(m.messageId ?? `${m.timestamp}\0${m.from}`) ); - return [...page.messages, ...older]; + return newMessages.length > 0 ? [...newMessages, ...prev] : prev; }); - setNextCursor(page.nextCursor); - setHasMore(page.hasMore); } catch { // best-effort } @@ -192,7 +188,14 @@ export const MessagesPanel = memo(function MessagesPanel({ beforeTimestamp: nextCursor, limit: PAGE_SIZE, }); - setFetchedMessages((prev) => [...prev, ...page.messages]); + // Dedup: only append messages we don't already have + setFetchedMessages((prev) => { + const existingIds = new Set(prev.map((m) => m.messageId ?? `${m.timestamp}\0${m.from}`)); + const newMessages = page.messages.filter( + (m) => !existingIds.has(m.messageId ?? `${m.timestamp}\0${m.from}`) + ); + return [...prev, ...newMessages]; + }); setNextCursor(page.nextCursor); setHasMore(page.hasMore); } catch { diff --git a/test/main/services/team/TeamDataService.test.ts b/test/main/services/team/TeamDataService.test.ts index 0e574462..d5cf430c 100644 --- a/test/main/services/team/TeamDataService.test.ts +++ b/test/main/services/team/TeamDataService.test.ts @@ -2145,4 +2145,97 @@ describe('TeamDataService', () => { }, }); }); + + describe('getMessagesPage', () => { + function createPaginationService(messages: Array<{ from: string; text: string; timestamp: string; messageId?: string; source?: string; leadSessionId?: string }>) { + return new TeamDataService( + { + listTeams: vi.fn(), + getConfig: vi.fn(async () => ({ + name: 'My team', + members: [{ name: 'team-lead', role: 'Lead' }], + leadSessionId: 'lead-1', + })), + } as never, + { getTasks: vi.fn(async () => []) } as never, + { + listInboxNames: vi.fn(async () => []), + getMessages: vi.fn(async () => + messages.map((m) => ({ ...m, read: true })) + ), + } as never, + {} as never, + {} as never, + { resolveMembers: vi.fn(() => []) } as never, + { getState: vi.fn(async () => ({ teamName: 'my-team', reviewers: [], tasks: {} })) } as never, + {} as never, + {} as never, + { readMessages: vi.fn(async () => []) } as never, + ); + } + + it('returns first page with cursor and hasMore', async () => { + const msgs = Array.from({ length: 5 }, (_, i) => ({ + from: 'alice', + text: `msg-${i}`, + timestamp: `2026-01-01T00:00:0${i}.000Z`, + messageId: `m${i}`, + source: 'inbox' as const, + })); + const service = createPaginationService(msgs); + const page = await service.getMessagesPage('my-team', { limit: 3 }); + + expect(page.messages).toHaveLength(3); + expect(page.hasMore).toBe(true); + expect(page.nextCursor).toBeTruthy(); + // Newest first + expect(page.messages[0].messageId).toBe('m4'); + }); + + it('cursor excludes already-seen messages without losing same-timestamp messages', async () => { + const msgs = [ + { from: 'a', text: '1', timestamp: '2026-01-01T00:00:02.000Z', messageId: 'x1' }, + { from: 'b', text: '2', timestamp: '2026-01-01T00:00:02.000Z', messageId: 'x2' }, + { from: 'c', text: '3', timestamp: '2026-01-01T00:00:01.000Z', messageId: 'x3' }, + ]; + const service = createPaginationService(msgs); + const page1 = await service.getMessagesPage('my-team', { limit: 1 }); + expect(page1.messages).toHaveLength(1); + expect(page1.hasMore).toBe(true); + + const page2 = await service.getMessagesPage('my-team', { + beforeTimestamp: page1.nextCursor!, + limit: 10, + }); + // Should get the remaining 2 messages, not lose the one with same timestamp + expect(page2.messages.length).toBeGreaterThanOrEqual(1); + const allIds = [...page1.messages, ...page2.messages].map((m) => m.messageId); + expect(new Set(allIds).size).toBe(allIds.length); // no duplicates + }); + + it('annotates slash command results in paginated path', async () => { + const msgs = [ + { + from: 'user', + text: '/cost', + timestamp: '2026-01-01T00:00:00.000Z', + messageId: 'cmd1', + source: 'user_sent', + leadSessionId: 'lead-1', + }, + { + from: 'team-lead', + text: 'Total cost: $1.05', + timestamp: '2026-01-01T00:00:01.000Z', + messageId: 'resp1', + source: 'lead_process', + leadSessionId: 'lead-1', + }, + ]; + const service = createPaginationService(msgs); + const page = await service.getMessagesPage('my-team', { limit: 10 }); + const result = page.messages.find((m) => m.messageId === 'resp1'); + expect(result?.messageKind).toBe('slash_command_result'); + }); + }); });