fix: pagination correctness and message enrichment
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.
This commit is contained in:
parent
bc7981f6b9
commit
5efc3dd63f
3 changed files with 164 additions and 17 deletions
|
|
@ -782,19 +782,70 @@ export class TeamDataService {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Sort newest-first
|
// Enrich: propagate leadSessionId to messages missing it (same as getTeamData)
|
||||||
messages.sort((a, b) => Date.parse(b.timestamp) - Date.parse(a.timestamp));
|
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) {
|
if (options.beforeTimestamp) {
|
||||||
const cursorMs = Date.parse(options.beforeTimestamp);
|
const [cursorTs, cursorId] = options.beforeTimestamp.split('|');
|
||||||
messages = messages.filter((m) => Date.parse(m.timestamp) < cursorMs);
|
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
|
// Paginate
|
||||||
const hasMore = messages.length > options.limit;
|
const hasMore = messages.length > options.limit;
|
||||||
const page = messages.slice(0, 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 };
|
return { messages: page, nextCursor, hasMore };
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -159,24 +159,20 @@ export const MessagesPanel = memo(function MessagesPanel({
|
||||||
})();
|
})();
|
||||||
}, [teamName]); // eslint-disable-line react-hooks/exhaustive-deps -- intentionally only on teamName change
|
}, [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(() => {
|
useEffect(() => {
|
||||||
if (!isTeamAlive && leadActivity !== 'active') return;
|
if (!isTeamAlive && leadActivity !== 'active') return;
|
||||||
const interval = setInterval(async () => {
|
const interval = setInterval(async () => {
|
||||||
try {
|
try {
|
||||||
const page = await api.teams.getMessagesPage(teamName, { limit: PAGE_SIZE });
|
const page = await api.teams.getMessagesPage(teamName, { limit: PAGE_SIZE });
|
||||||
setFetchedMessages((prev) => {
|
setFetchedMessages((prev) => {
|
||||||
// Merge: keep older messages that aren't in the new page
|
const existingIds = new Set(prev.map((m) => m.messageId ?? `${m.timestamp}\0${m.from}`));
|
||||||
const newIds = new Set(page.messages.map((m) => m.messageId ?? m.timestamp));
|
const newMessages = page.messages.filter(
|
||||||
const older = prev.filter(
|
(m) => !existingIds.has(m.messageId ?? `${m.timestamp}\0${m.from}`)
|
||||||
(m) =>
|
|
||||||
!newIds.has(m.messageId ?? m.timestamp) &&
|
|
||||||
!page.messages.some((n) => n.timestamp === m.timestamp && n.from === m.from)
|
|
||||||
);
|
);
|
||||||
return [...page.messages, ...older];
|
return newMessages.length > 0 ? [...newMessages, ...prev] : prev;
|
||||||
});
|
});
|
||||||
setNextCursor(page.nextCursor);
|
|
||||||
setHasMore(page.hasMore);
|
|
||||||
} catch {
|
} catch {
|
||||||
// best-effort
|
// best-effort
|
||||||
}
|
}
|
||||||
|
|
@ -192,7 +188,14 @@ export const MessagesPanel = memo(function MessagesPanel({
|
||||||
beforeTimestamp: nextCursor,
|
beforeTimestamp: nextCursor,
|
||||||
limit: PAGE_SIZE,
|
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);
|
setNextCursor(page.nextCursor);
|
||||||
setHasMore(page.hasMore);
|
setHasMore(page.hasMore);
|
||||||
} catch {
|
} catch {
|
||||||
|
|
|
||||||
|
|
@ -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');
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue