mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-sessions): strip pastes, restore known payloads, handle mirrored clear
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
522b4bc67e
commit
e318a99255
@@ -651,7 +651,10 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. -->
|
||||
{/snippet}
|
||||
</Popover>
|
||||
<Button
|
||||
title="New chat"
|
||||
title={aiChatManager.mirroringRemoteRun
|
||||
? 'This session is running in another tab'
|
||||
: 'New chat'}
|
||||
disabled={aiChatManager.mirroringRemoteRun}
|
||||
on:click={() => {
|
||||
saveAndClear()
|
||||
}}
|
||||
|
||||
@@ -4093,6 +4093,22 @@ export class AIChatManager {
|
||||
}
|
||||
}
|
||||
|
||||
/** Point this manager at a conversation that has no stored record yet, with
|
||||
* nothing in it. For a tab catching up on a chat rotation another tab made
|
||||
* ("/clear", or a turn that rolled back to empty): its transcript and model
|
||||
* history belong to the conversation just left, and carrying them into the
|
||||
* new id would send that history back to the model and persist it there. */
|
||||
adoptEmptyChat = (chatId: string) => {
|
||||
this.historyManager.setCurrentChatId(chatId)
|
||||
this.displayMessages = []
|
||||
this.messages = []
|
||||
this.contextUsage = undefined
|
||||
this.clearBackgroundJobs()
|
||||
if (this.modifiedItems) this.modifiedItems = new SvelteSet()
|
||||
this.#syncMessageFiles()
|
||||
this.planMode.resetRound()
|
||||
}
|
||||
|
||||
private syncArtifactsSession = () => {
|
||||
void this.artifacts.setSession(this.isSessionChat ? this.sessionId : undefined)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import { withRestoredPayloads, withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import type { DisplayMessage } from '$lib/components/copilot/chat/shared'
|
||||
|
||||
// Frames go out several times a second for the whole turn, so anything left in
|
||||
@@ -15,7 +15,8 @@ describe('withoutHeavyPayloads', () => {
|
||||
images: [
|
||||
{ dataUrl: 'data:image/png;base64,AAAA', mediaType: 'image/png', name: 'shot.png' }
|
||||
],
|
||||
files: [{ name: 'notes.md', content: 'a very long pasted file', id: 'f1' }]
|
||||
files: [{ name: 'notes.md', content: 'a very long pasted file', id: 'f1' }],
|
||||
pastes: [{ id: 1, lines: 400, content: 'a very long collapsed paste' }]
|
||||
},
|
||||
{ role: 'tool', content: 'took a screenshot', imageUrl: 'data:image/png;base64,BBBB' }
|
||||
] as unknown as DisplayMessage[]
|
||||
@@ -24,6 +25,9 @@ describe('withoutHeavyPayloads', () => {
|
||||
|
||||
expect(JSON.stringify(stripped)).not.toContain('data:image')
|
||||
expect(JSON.stringify(stripped)).not.toContain('a very long pasted file')
|
||||
expect(JSON.stringify(stripped)).not.toContain('a very long collapsed paste')
|
||||
// The chip renders from the line count, so that has to survive.
|
||||
expect((stripped[0] as any).pastes).toEqual([{ id: 1, lines: 400, content: '' }])
|
||||
// The file chip is labelled from the name, so that has to survive.
|
||||
expect((stripped[0] as any).files).toEqual([{ name: 'notes.md', content: '', id: 'f1' }])
|
||||
// Images are dropped whole: the bubble renders one <img> per entry with no
|
||||
@@ -37,3 +41,29 @@ describe('withoutHeavyPayloads', () => {
|
||||
expect(withoutHeavyPayloads(messages)[0]).toBe(messages[0])
|
||||
})
|
||||
})
|
||||
|
||||
// A frame carries the last several messages, but only the newest are new to the
|
||||
// watcher; the rest it already holds complete from IndexedDB.
|
||||
describe('withRestoredPayloads', () => {
|
||||
it('keeps attachments the watcher already had', () => {
|
||||
const local = {
|
||||
role: 'user',
|
||||
content: 'look at this',
|
||||
images: [{ dataUrl: 'data:image/png;base64,AAAA', mediaType: 'image/png' }],
|
||||
files: [{ name: 'notes.md', content: 'the real content', id: 'f1' }],
|
||||
pastes: [{ id: 1, lines: 400, content: 'the real paste' }]
|
||||
} as unknown as DisplayMessage
|
||||
|
||||
const [merged] = withRestoredPayloads(withoutHeavyPayloads([local]), () => local)
|
||||
|
||||
expect(merged).toEqual(local)
|
||||
})
|
||||
|
||||
it('leaves a message the watcher does not have yet stripped', () => {
|
||||
const incoming = [
|
||||
{ role: 'user', content: 'brand new', files: [{ name: 'a.md', content: '', id: 'f9' }] }
|
||||
] as unknown as DisplayMessage[]
|
||||
|
||||
expect(withRestoredPayloads(incoming, () => undefined)).toEqual(incoming)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,31 +1,94 @@
|
||||
import type { DisplayMessage } from '$lib/components/copilot/chat/shared'
|
||||
|
||||
/**
|
||||
* Drop the attachment bytes from the transcript a mirror frame carries.
|
||||
* The attachment payloads a mirror frame does not carry, and how to put them
|
||||
* back. A frame goes out several times a second for the whole turn, and the
|
||||
* message holding an image or a pasted file sits in its tail unchanged
|
||||
* throughout — so shipping the payloads means re-cloning and re-broadcasting
|
||||
* the same megabytes on every tick (the caps allow roughly 16MB of base64 image
|
||||
* plus 5MB of file content on a single message, before pastes).
|
||||
*
|
||||
* A frame goes out several times a second for the whole turn, and the message
|
||||
* that holds an image or a pasted file sits in its tail unchanged throughout —
|
||||
* so shipping the payloads means re-cloning and re-broadcasting the same
|
||||
* megabytes on every tick (the caps allow roughly 16MB of base64 image plus 5MB
|
||||
* of file content on a single message). Watchers render the transcript without
|
||||
* them and receive the real thing when the turn ends and they re-read the
|
||||
* record the driving tab saved.
|
||||
* Every heavy field is listed here rather than at its own call site: they are
|
||||
* spread across the message union, and one that goes unlisted silently
|
||||
* reintroduces the whole regression.
|
||||
*/
|
||||
|
||||
/** Emptied in place, keeping the sibling fields the chip renders from. */
|
||||
const BLANKED_ITEM_FIELDS = [
|
||||
['files', 'content'],
|
||||
['pastes', 'content']
|
||||
] as const
|
||||
|
||||
/** Dropped whole: the bubble renders one <img> per entry with no per-image
|
||||
* guard, so an emptied url would show a broken image where the real one is
|
||||
* about to appear. */
|
||||
const DROPPED_LIST_FIELDS = ['images'] as const
|
||||
|
||||
/** Emptied outright; guarded at the render site, so it renders nothing. */
|
||||
const BLANKED_FIELDS = ['imageUrl'] as const
|
||||
|
||||
function isHeavy(message: DisplayMessage): boolean {
|
||||
const m = message as Record<string, any>
|
||||
return (
|
||||
DROPPED_LIST_FIELDS.some((f) => m[f]?.length) ||
|
||||
BLANKED_ITEM_FIELDS.some(([f]) => m[f]?.length) ||
|
||||
BLANKED_FIELDS.some((f) => m[f])
|
||||
)
|
||||
}
|
||||
|
||||
/** Strip the bytes out of a transcript bound for a mirror frame. */
|
||||
export function withoutHeavyPayloads(messages: DisplayMessage[]): DisplayMessage[] {
|
||||
return messages.map((message) => {
|
||||
const images = 'images' in message ? message.images : undefined
|
||||
const files = 'files' in message ? message.files : undefined
|
||||
const imageUrl = 'imageUrl' in message ? message.imageUrl : undefined
|
||||
if (!images?.length && !files?.length && !imageUrl) return message
|
||||
const stripped: Record<string, unknown> = { ...message }
|
||||
// Dropped rather than blanked: the bubble renders one <img> per entry with
|
||||
// no per-image guard, so an emptied dataUrl would show a broken image where
|
||||
// the real one is about to appear.
|
||||
if (images?.length) delete stripped.images
|
||||
// Kept, minus the bytes: the chip is labelled from the name.
|
||||
if (files?.length) stripped.files = files.map((f) => ({ ...f, content: '' }))
|
||||
// Guarded at the render site, so an empty string renders nothing.
|
||||
if (imageUrl) stripped.imageUrl = ''
|
||||
if (!isHeavy(message)) return message
|
||||
const stripped: Record<string, any> = { ...message }
|
||||
for (const f of DROPPED_LIST_FIELDS) delete stripped[f]
|
||||
for (const [f, item] of BLANKED_ITEM_FIELDS) {
|
||||
if (stripped[f]?.length) stripped[f] = stripped[f].map((v: any) => ({ ...v, [item]: '' }))
|
||||
}
|
||||
for (const f of BLANKED_FIELDS) if (stripped[f]) stripped[f] = ''
|
||||
return stripped as DisplayMessage
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Put back the payloads a watcher already holds.
|
||||
*
|
||||
* A frame carries the last several messages, but only the newest one or two are
|
||||
* ever new to the watcher — the rest it loaded from IndexedDB with attachments
|
||||
* intact. Overwriting those with the stripped copies makes a screenshot from an
|
||||
* earlier turn vanish the moment another tab starts a turn, and stay gone until
|
||||
* the turn ends. Positional, which is what the frame's own indexing already
|
||||
* assumes: a transcript only grows within a turn, and a rewrite (compaction)
|
||||
* forces a full frame instead of a tail.
|
||||
*/
|
||||
export function withRestoredPayloads(
|
||||
incoming: DisplayMessage[],
|
||||
localAt: (offset: number) => DisplayMessage | undefined
|
||||
): DisplayMessage[] {
|
||||
return incoming.map((message, i) => {
|
||||
const local = localAt(i) as Record<string, any> | undefined
|
||||
if (!local || local.role !== (message as any).role) return message
|
||||
const merged: Record<string, any> = { ...message }
|
||||
let changed = false
|
||||
for (const f of DROPPED_LIST_FIELDS) {
|
||||
if (merged[f] === undefined && local[f]?.length) {
|
||||
merged[f] = local[f]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
for (const [f, item] of BLANKED_ITEM_FIELDS) {
|
||||
if (!merged[f]?.length || !local[f]?.length) continue
|
||||
merged[f] = merged[f].map((v: any, j: number) =>
|
||||
v[item] === '' && local[f][j]?.[item] ? { ...v, [item]: local[f][j][item] } : v
|
||||
)
|
||||
changed = true
|
||||
}
|
||||
for (const f of BLANKED_FIELDS) {
|
||||
if (merged[f] === '' && local[f]) {
|
||||
merged[f] = local[f]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
return (changed ? merged : message) as DisplayMessage
|
||||
})
|
||||
}
|
||||
|
||||
@@ -93,7 +93,7 @@ import { getNonStreamingMetadataCompletion } from '$lib/components/copilot/lib'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { pendingUserAction, type DisplayMessage } from '$lib/components/copilot/chat/shared'
|
||||
import type { ChatCompletionMessageParam } from 'openai/resources/index.mjs'
|
||||
import { withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import { withRestoredPayloads, withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import {
|
||||
broadcastMirror,
|
||||
broadcastTurnEnd,
|
||||
@@ -1039,7 +1039,9 @@ function mirrorSnapshotOf(sessionId: string, full: boolean): MirrorSnapshot | un
|
||||
sessionId,
|
||||
chatId: m.historyManager.getCurrentChatId(),
|
||||
baseIndex,
|
||||
tail: withoutHeavyPayloads($state.snapshot(m.displayMessages.slice(baseIndex)) as DisplayMessage[]),
|
||||
tail: withoutHeavyPayloads(
|
||||
$state.snapshot(m.displayMessages.slice(baseIndex)) as DisplayMessage[]
|
||||
),
|
||||
total,
|
||||
loading: m.loading,
|
||||
currentReply: m.currentReply,
|
||||
@@ -1085,7 +1087,8 @@ function applyMirror(msg: MirrorMsg): void {
|
||||
if (!runtime) return
|
||||
const m = runtime.manager
|
||||
const onSameChat = m.historyManager.getCurrentChatId() === msg.chatId
|
||||
const prefixFits = m.displayMessages.length >= msg.baseIndex && m.displayMessages.length <= msg.total
|
||||
const prefixFits =
|
||||
m.displayMessages.length >= msg.baseIndex && m.displayMessages.length <= msg.total
|
||||
if (msg.baseIndex > 0 && !(onSameChat && prefixFits)) {
|
||||
// Nothing here can host this tail: this tab joined mid-run, or the driver
|
||||
// rotated to a chat it isn't on. Ask for the whole transcript instead of
|
||||
@@ -1099,8 +1102,12 @@ function applyMirror(msg: MirrorMsg): void {
|
||||
m.historyManager.setCurrentChatId(msg.chatId)
|
||||
setSessionChatId(msg.sessionId, msg.chatId)
|
||||
}
|
||||
// The frame's tail arrives stripped of attachment bytes; anything this tab
|
||||
// already holds complete stays complete, so an earlier turn's screenshot does
|
||||
// not blink out for the length of someone else's turn.
|
||||
const tail = withRestoredPayloads(msg.tail, (i) => m.displayMessages[msg.baseIndex + i])
|
||||
m.displayMessages =
|
||||
msg.baseIndex === 0 ? msg.tail : [...m.displayMessages.slice(0, msg.baseIndex), ...msg.tail]
|
||||
msg.baseIndex === 0 ? tail : [...m.displayMessages.slice(0, msg.baseIndex), ...tail]
|
||||
// The frame carries the rendered transcript but not the API-format history,
|
||||
// so this manager is now holding a mismatched pair. Flag it: the save paths
|
||||
// that run outside a turn would otherwise write that pair over the record the
|
||||
@@ -1140,10 +1147,19 @@ async function applyTurnEnd(sessionId: string, chatId: string, committed: boolea
|
||||
m.loadingLabel = undefined
|
||||
m.compacting = false
|
||||
const id = chatId || m.historyManager.getCurrentChatId()
|
||||
// `refresh`: this is the same conversation caught up from the store, so a
|
||||
// message queued here while the other tab held the session is still meant for
|
||||
// it — a plain load would drop it on the floor instead of sending it below.
|
||||
if (id && (await m.historyManager.reloadChat(id))) await m.loadPastChat(id, { refresh: true })
|
||||
if (id && (await m.historyManager.reloadChat(id))) {
|
||||
// `refresh`: this is the same conversation caught up from the store, so a
|
||||
// message queued here while the other tab held the session is still meant
|
||||
// for it — a plain load would drop it on the floor instead of sending it.
|
||||
await m.loadPastChat(id, { refresh: true })
|
||||
} else if (id && id !== m.historyManager.getCurrentChatId()) {
|
||||
// The driver rotated to a chat with no record yet: it ran "/clear", or its
|
||||
// turn rolled back to nothing. Either way this tab's transcript and model
|
||||
// history belong to the conversation just left, and keeping them would
|
||||
// send that history under the new id on the next turn.
|
||||
m.adoptEmptyChat(id)
|
||||
setSessionChatId(sessionId, id)
|
||||
}
|
||||
// Anything typed here while the other tab held the session was queued rather
|
||||
// than sent. Send it only after a turn that landed, which is the rule a turn
|
||||
// follows locally: firing it into a failed turn, or into the gap left by a
|
||||
|
||||
Reference in New Issue
Block a user