diff --git a/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts b/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts index b3347dd998..c3ec05b09b 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts @@ -934,6 +934,7 @@ export class FlowChatManager { // temp rows that were standing in for it. const response: ChatMessage[] = [] let afterSeq = this.getLastPersistedMessageSeq(conversationId) + let readWhole = false for (let page = 0; page < POLL_MAX_PAGES; page++) { const batch = await FlowConversationsService.listConversationMessages({ workspace: this.#workspace()!, @@ -946,10 +947,16 @@ export class FlowChatManager { // poll outlives its abort — either would put a forgotten flow's rows back. if (startedIn !== this.#generation) return response.push(...batch) - if (batch.length < POLL_PAGE_SIZE) break + if (batch.length < POLL_PAGE_SIZE) { + readWhole = true + break + } const furthest = Math.max(...batch.map((m) => m.created_seq)) // A page that moved nothing would ask for the same rows forever. - if (afterSeq !== undefined && furthest <= afterSeq) break + if (afterSeq !== undefined && furthest <= afterSeq) { + readWhole = true + break + } afterSeq = furthest } @@ -967,9 +974,15 @@ export class FlowChatManager { } } + if (!readWhole) { + // The cap is a guard against a cursor that stops advancing, not a reason to + // believe the conversation ends here. Sweeping now would drop the rows standing + // in for what was never read — the failure this paging exists to prevent. + console.warn(`Stopped reading conversation ${conversationId} after ${POLL_MAX_PAGES} pages`) + } // Only remove temporary messages when explicitly requested (e.g., after job completion) // During streaming, we keep temp messages to avoid them disappearing due to race conditions - if (options?.removeTempMessages) { + if (options?.removeTempMessages && readWhole) { this.#rowsById[conversationId] = this.#rowsOf(conversationId).filter( (msg) => !msg.id.startsWith('temp-') || msg.message_type === 'user' ) diff --git a/frontend/src/lib/components/flows/conversations/FlowChatManager.test.ts b/frontend/src/lib/components/flows/conversations/FlowChatManager.test.ts index 5bc13e0f2e..b6782fabf2 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChatManager.test.ts +++ b/frontend/src/lib/components/flows/conversations/FlowChatManager.test.ts @@ -200,6 +200,10 @@ describe('reading a turn longer than one page', () => { expect(manager.messages).toHaveLength(62) expect(manager.messages.at(-1)?.content).toBe('row 62') + // The second read starts where the first stopped; a cursor that did not move would + // re-fetch the same page and still look right from the rows alone. + const calls = vi.mocked(FlowConversationsService.listConversationMessages).mock.calls + expect((calls[1][0] as any).afterSeq).toBe(50) }) }) diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts index 2392fb5be1..591c219d40 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts @@ -87,6 +87,23 @@ describe('a send whose attachments are still uploading', () => { }) // Stop has no job to cancel yet, so it has to be honoured when the upload lands — + // otherwise the run starts and the reader watches a message they took back execute. + it('does not run after a Stop pressed while it uploaded', async () => { + const manager = stubManager('a') + const chatHost = host(manager) + vi.mocked(HelpersService.fileUpload).mockImplementation(async () => { + chatHost.cancel() + return { file_key: 'k' } as any + }) + + const started = await chatHost.sendRequest({ + instructions: 'stop me', + blobs: [anAttachment] as any + }) + + expect(started).toBe(false) + expect(manager.sendMessage).not.toHaveBeenCalled() + }) /** * Two files can arrive under one name. Keyed on the name alone they race to the same @@ -122,24 +139,6 @@ describe('a send whose attachments are still uploading', () => { expect(keys.every((k) => k.endsWith('/report.pdf'))).toBe(true) }) - // otherwise the run starts and the reader watches a message they took back execute. - it('does not run after a Stop pressed while it uploaded', async () => { - const manager = stubManager('a') - const chatHost = host(manager) - vi.mocked(HelpersService.fileUpload).mockImplementation(async () => { - chatHost.cancel() - return { file_key: 'k' } as any - }) - - const started = await chatHost.sendRequest({ - instructions: 'stop me', - blobs: [anAttachment] as any - }) - - expect(started).toBe(false) - expect(manager.sendMessage).not.toHaveBeenCalled() - }) - // The first message of a chat has no conversation until one is made. Left to // `sendMessage` afterwards, nothing in this window had an id to work with. it('creates the conversation before uploading, so a first message can be stopped too', async () => {