mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(chat): make the page cap a state the transcript can survive
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
81d843a24c
commit
ad847bfa78
@@ -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'
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user