diff --git a/apps/web/src/domains/chat/api/conversations.test.ts b/apps/web/src/domains/chat/api/conversations.test.ts index cc62b81c7ce..2f2f4d0a2c9 100644 --- a/apps/web/src/domains/chat/api/conversations.test.ts +++ b/apps/web/src/domains/chat/api/conversations.test.ts @@ -1,6 +1,7 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; -import { parseConversation } from "@/domains/chat/api/conversations.js"; +import { client } from "@/domains/chat/api/client.js"; +import { listConversations, parseConversation } from "@/domains/chat/api/conversations.js"; describe("parseConversation — originChannel plumbing", () => { test("returns null for non-object input", () => { @@ -133,6 +134,135 @@ describe("parseConversation — originChannel plumbing", () => { }); }); +describe("parseConversation — displayOrder", () => { + test("captures numeric displayOrder for drag-reordered conversations", () => { + const parsed = parseConversation({ + conversationKey: "conv-pinned", + isPinned: true, + displayOrder: 3, + }); + expect(parsed?.displayOrder).toBe(3); + }); + + test("leaves displayOrder undefined when the field is absent", () => { + const parsed = parseConversation({ conversationKey: "conv-fresh" }); + expect(parsed?.displayOrder).toBeUndefined(); + }); + + test("treats non-finite displayOrder as missing", () => { + expect( + parseConversation({ + conversationKey: "c1", + displayOrder: Number.NaN, + })?.displayOrder, + ).toBeUndefined(); + expect( + parseConversation({ + conversationKey: "c2", + displayOrder: "0", + })?.displayOrder, + ).toBeUndefined(); + }); +}); + +describe("listConversations — pagination", () => { + const originalGet = client.get; + type GetOptions = { + query?: Record; + }; + + type Page = { + conversations: Array<{ conversationKey: string }>; + hasMore?: boolean; + }; + + function setupPagedResponses(pages: { + foreground: Page[]; + background?: Page[]; + }): { calls: Array<{ url: unknown; query: Record | undefined }> } { + const calls: Array<{ url: unknown; query: Record | undefined }> = []; + const foregroundQueue = [...pages.foreground]; + const backgroundQueue = [...(pages.background ?? [{ conversations: [] }])]; + client.get = mock( + async (options: GetOptions & { url?: unknown }) => { + calls.push({ url: options.url, query: options.query }); + const isBackground = options.query?.conversationType === "background"; + const queue = isBackground ? backgroundQueue : foregroundQueue; + const next = queue.shift() ?? { conversations: [], hasMore: false }; + return { + data: next, + error: null, + response: new Response(null, { status: 200 }), + }; + }, + ) as typeof client.get; + return { calls }; + } + + afterEach(() => { + client.get = originalGet; + }); + + test("loops over pages until hasMore is false (>50 conversations preserved)", async () => { + const page1Items = Array.from({ length: 50 }, (_, i) => ({ + conversationKey: `foreground-${i}`, + })); + const page2Items = Array.from({ length: 30 }, (_, i) => ({ + conversationKey: `foreground-${50 + i}`, + })); + const { calls } = setupPagedResponses({ + foreground: [ + { conversations: page1Items, hasMore: true }, + { conversations: page2Items, hasMore: false }, + ], + }); + + const result = await listConversations("assistant-1"); + + expect(result).toHaveLength(80); + expect(result.at(0)?.conversationKey).toBe("foreground-0"); + expect(result.at(-1)?.conversationKey).toBe("foreground-79"); + // 2 foreground pages + 1 background page (empty by default). Foreground + // and background fetch in parallel via Promise.allSettled, so filter + // before asserting page offsets. + expect(calls).toHaveLength(3); + const foregroundCalls = calls.filter( + (c) => c.query?.conversationType === undefined, + ); + expect(foregroundCalls).toHaveLength(2); + expect(foregroundCalls[0]?.query).toMatchObject({ limit: 50, offset: 0 }); + expect(foregroundCalls[1]?.query).toMatchObject({ limit: 50, offset: 50 }); + }); + + test("stops on the first page when hasMore is false or absent", async () => { + const { calls } = setupPagedResponses({ + foreground: [ + { conversations: [{ conversationKey: "only-one" }] }, + ], + }); + + const result = await listConversations("assistant-1"); + + expect(result).toHaveLength(1); + // 1 foreground + 1 background + expect(calls).toHaveLength(2); + }); + + test("does not loop forever on hasMore=true with empty page", async () => { + const { calls } = setupPagedResponses({ + foreground: [ + { conversations: [{ conversationKey: "a" }], hasMore: true }, + { conversations: [], hasMore: true }, + ], + }); + + const result = await listConversations("assistant-1"); + + expect(result).toHaveLength(1); + expect(calls).toHaveLength(3); // 2 foreground + 1 background + }); +}); + describe("parseConversation — Slack channel binding", () => { test("preserves Slack channel binding with id, name, and link", () => { const parsed = parseConversation({ diff --git a/apps/web/src/domains/chat/api/conversations.ts b/apps/web/src/domains/chat/api/conversations.ts index 1c3c92bf5ad..66265d28ecd 100644 --- a/apps/web/src/domains/chat/api/conversations.ts +++ b/apps/web/src/domains/chat/api/conversations.ts @@ -37,6 +37,14 @@ export interface Conversation { isPinned?: boolean; conversationType?: string; scheduleJobId?: string; + /** + * Server-provided sort order for pinned and custom-group buckets. Set when + * the user has drag-reordered the conversation; absent for conversations + * that have never been reordered. Consumers (see `groupConversations`) + * should sort pinned / custom-group buckets by this field so the user's + * order is preserved across reloads. + */ + displayOrder?: number; channelBinding?: ConversationChannelBinding; /** * Channel of origin for this conversation, e.g. `"slack"`, `"telegram"`, @@ -74,6 +82,7 @@ export interface ConversationSlackThread { interface ListConversationsResponse { conversations: Conversation[]; + hasMore?: boolean; } interface ConversationAttentionPayload { @@ -233,46 +242,88 @@ export function parseConversation(raw: unknown): Conversation | null { typeof record.conversationType === "string" ? record.conversationType : undefined, scheduleJobId: typeof record.scheduleJobId === "string" ? record.scheduleJobId : undefined, + displayOrder: + typeof record.displayOrder === "number" && Number.isFinite(record.displayOrder) + ? record.displayOrder + : undefined, channelBinding: parsedChannelBinding, originChannel, }; } +/** + * Daemon default page size for `/v1/assistants/{id}/conversations/`. Used + * as our explicit page size so pagination state is predictable across daemon + * versions. See `ConversationListRequest` in + * `assistant/src/daemon/message-types/conversations.ts`. + */ +const CONVERSATION_LIST_PAGE_SIZE = 50; + +/** + * Safety cap on the pagination loop. Multiplied by `CONVERSATION_LIST_PAGE_SIZE` + * this allows for 10,000 conversations of a single type — far above any + * realistic user count, but bounded so a malformed `hasMore` from the server + * can't spin forever. + */ +const CONVERSATION_LIST_MAX_PAGES = 200; + async function fetchConversationList( assistantId: string, conversationType?: "background", ): Promise { - const { data, error, response } = await client.get< - ListConversationsResponse, - unknown - >({ - ...SDK_BASE_OPTIONS, - url: "/v1/assistants/{assistant_id}/conversations/", - path: { assistant_id: assistantId }, - query: conversationType ? { conversationType } : undefined, - throwOnError: false, - }); - assertHasResponse(response, error, "Failed to list conversations."); - if (!response.ok) { - const msg = extractErrorMessage(error, response, "Failed to list conversations."); - throw new ApiError(response.status, msg); + const all: Conversation[] = []; + + for (let page = 0; page < CONVERSATION_LIST_MAX_PAGES; page++) { + const offset = page * CONVERSATION_LIST_PAGE_SIZE; + const { data, error, response } = await client.get< + ListConversationsResponse, + unknown + >({ + ...SDK_BASE_OPTIONS, + url: "/v1/assistants/{assistant_id}/conversations/", + path: { assistant_id: assistantId }, + query: { + ...(conversationType ? { conversationType } : {}), + limit: CONVERSATION_LIST_PAGE_SIZE, + offset, + }, + throwOnError: false, + }); + assertHasResponse(response, error, "Failed to list conversations."); + if (!response.ok) { + const msg = extractErrorMessage(error, response, "Failed to list conversations."); + throw new ApiError(response.status, msg); + } + const payload = + data && typeof data === "object" && !Array.isArray(data) + ? (data as unknown as { + conversations?: unknown; + sessions?: unknown; + hasMore?: unknown; + }) + : null; + const rawItems = Array.isArray(payload?.conversations) + ? payload.conversations + : Array.isArray(payload?.sessions) + ? payload.sessions + : []; + + const pageItems = rawItems + .map((conversation) => parseConversation(conversation)) + .filter((conversation): conversation is Conversation => conversation !== null); + + all.push(...pageItems); + + const hasMore = + typeof payload?.hasMore === "boolean" ? payload.hasMore : false; + if (!hasMore) break; + + // Defensive: a malformed `hasMore: true` with an empty page would loop + // forever. Treat an empty page as end-of-list regardless of `hasMore`. + if (pageItems.length === 0) break; } - const payload = - data && typeof data === "object" && !Array.isArray(data) - ? (data as unknown as { - conversations?: unknown; - sessions?: unknown; - }) - : null; - const rawItems = Array.isArray(payload?.conversations) - ? payload.conversations - : Array.isArray(payload?.sessions) - ? payload.sessions - : []; - - return rawItems - .map((conversation) => parseConversation(conversation)) - .filter((conversation): conversation is Conversation => conversation !== null); + + return all; } /** diff --git a/apps/web/src/domains/chat/utils/group-conversations.test.ts b/apps/web/src/domains/chat/utils/group-conversations.test.ts index 991781a5bc5..dd5fa112a34 100644 --- a/apps/web/src/domains/chat/utils/group-conversations.test.ts +++ b/apps/web/src/domains/chat/utils/group-conversations.test.ts @@ -481,3 +481,114 @@ describe("groupConversations · custom group routing", () => { expect(result.customGroups[0]?.id).toBe("grp-work"); }); }); + +describe("groupConversations · displayOrder for pinned and custom groups", () => { + test("pinned bucket sorts by displayOrder ascending (user-set order)", () => { + // Note: input is provided newest-first by lastMessageAt to confirm the + // pinned sort overrides recency, matching the bug described in LUM-1619. + const result = groupConversations([ + makeConversation({ + conversationKey: "b", + isPinned: true, + displayOrder: 1, + lastMessageAt: "2024-01-03T00:00:00.000Z", + }), + makeConversation({ + conversationKey: "a", + isPinned: true, + displayOrder: 0, + lastMessageAt: "2024-01-02T00:00:00.000Z", + }), + makeConversation({ + conversationKey: "c", + isPinned: true, + displayOrder: 2, + lastMessageAt: "2024-01-01T00:00:00.000Z", + }), + ]); + expect(result.pinned.map((c) => c.conversationKey)).toEqual([ + "a", + "b", + "c", + ]); + }); + + test("pinned conversations without displayOrder fall back to lastMessageAt desc", () => { + const result = groupConversations([ + makeConversation({ + conversationKey: "old", + isPinned: true, + lastMessageAt: "2024-01-01T00:00:00.000Z", + }), + makeConversation({ + conversationKey: "new", + isPinned: true, + lastMessageAt: "2024-01-05T00:00:00.000Z", + }), + ]); + expect(result.pinned.map((c) => c.conversationKey)).toEqual([ + "new", + "old", + ]); + }); + + test("displayOrder rows come before rows without displayOrder", () => { + const result = groupConversations([ + makeConversation({ + conversationKey: "no-order-newer", + isPinned: true, + lastMessageAt: "2024-01-10T00:00:00.000Z", + }), + makeConversation({ + conversationKey: "ordered-0", + isPinned: true, + displayOrder: 0, + lastMessageAt: "2024-01-01T00:00:00.000Z", + }), + ]); + expect(result.pinned.map((c) => c.conversationKey)).toEqual([ + "ordered-0", + "no-order-newer", + ]); + }); + + test("custom group conversations sort by displayOrder", () => { + const groups: ConversationGroup[] = [ + { + id: "grp-work", + name: "Work", + sortPosition: 0, + isSystemGroup: false, + }, + ]; + const result = groupConversations( + [ + makeConversation({ + conversationKey: "z", + groupId: "grp-work", + displayOrder: 2, + lastMessageAt: "2024-01-03T00:00:00.000Z", + }), + makeConversation({ + conversationKey: "x", + groupId: "grp-work", + displayOrder: 0, + lastMessageAt: "2024-01-01T00:00:00.000Z", + }), + makeConversation({ + conversationKey: "y", + groupId: "grp-work", + displayOrder: 1, + lastMessageAt: "2024-01-02T00:00:00.000Z", + }), + ], + { groups, customGroupsEnabled: true }, + ); + const work = result.customGroups.find((g) => g.id === "grp-work"); + expect(work?.conversations.map((c) => c.conversationKey)).toEqual([ + "x", + "y", + "z", + ]); + }); +}); diff --git a/apps/web/src/domains/chat/utils/group-conversations.ts b/apps/web/src/domains/chat/utils/group-conversations.ts index 766740e65bc..b15372468ca 100644 --- a/apps/web/src/domains/chat/utils/group-conversations.ts +++ b/apps/web/src/domains/chat/utils/group-conversations.ts @@ -149,6 +149,25 @@ function parseLastMessageAt(conversation: Conversation): number { return Number.isFinite(parsed) ? parsed : 0; } +/** + * Comparator for buckets where the user can manually drag-reorder rows + * (pinned, custom groups). Conversations with a server-provided + * `displayOrder` come first in ascending order; ties and rows without + * `displayOrder` fall back to `lastMessageAt` newest-first so freshly-pinned + * conversations land near the top until the server assigns them an order. + */ +function compareByDisplayOrder(a: Conversation, b: Conversation): number { + const aOrder = a.displayOrder; + const bOrder = b.displayOrder; + if (aOrder != null && bOrder != null) { + if (aOrder !== bOrder) return aOrder - bOrder; + return parseLastMessageAt(b) - parseLastMessageAt(a); + } + if (aOrder != null) return -1; + if (bOrder != null) return 1; + return parseLastMessageAt(b) - parseLastMessageAt(a); +} + /** * True when a `groupId` refers to a non-system (custom) group. * System groups use a `"system:"` prefix (e.g. `"system:pinned"`). @@ -233,9 +252,16 @@ export function groupConversations( const sortedSlack = slack.slice().sort((a, b) => { return parseLastMessageAt(b) - parseLastMessageAt(a); }); + // Pinned + custom groups honor `displayOrder` (set when the user + // drag-reorders). Any global resort by recency at this level would + // override the user's custom order — see LUM-1619. + const sortedPinned = pinned.slice().sort(compareByDisplayOrder); + for (const bucket of customGroupsList) { + bucket.conversations.sort(compareByDisplayOrder); + } return { - pinned, + pinned: sortedPinned, scheduled, background, slack: sortedSlack,