From ad847bfa782eb97a70eff8af5cbd546a8aacd9e8 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Tue, 15 Sep 2026 17:49:18 +0200 Subject: [PATCH] fix(chat): make the page cap a state the transcript can survive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stopping at the cap left the conversation looking fully read, and the sweep then dropped the temp rows standing in for everything beyond it — the same vanished answer the paging was added to prevent, one order of magnitude up. The cap is a guard against a cursor that stops advancing, not evidence the conversation ends there, so a poll that hit it keeps its temp rows and says so. The paging test now pins the cursor as well as the rows: a second read that asked for the same page again would have looked right from the rows alone. Moves the attachment test out of the middle of another test's comment, which it had split across two unrelated cases. Co-Authored-By: Claude Opus 5 (1M context) --- .../conversations/FlowChatManager.svelte.ts | 19 ++++++++-- .../conversations/FlowChatManager.test.ts | 4 +++ .../conversations/flowChatViewHost.test.ts | 35 +++++++++---------- 3 files changed, 37 insertions(+), 21 deletions(-) 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 () => {