diff --git a/apps/desktop/src/app/chat/hooks/use-session-changes.test.ts b/apps/desktop/src/app/chat/hooks/use-session-changes.test.ts index c47951192f7b7..5d065e1bb9ba5 100644 --- a/apps/desktop/src/app/chat/hooks/use-session-changes.test.ts +++ b/apps/desktop/src/app/chat/hooks/use-session-changes.test.ts @@ -629,3 +629,60 @@ describe('reconnect-seam zombie optimistic rows (severed message.complete stamp) }) }) +describe('role-aware tail-first stamping (stale zombie + fresh turn, live 2026-07-15)', () => { + function textMessage2(id: string, role: ChatMessage['role'], text: string, pending = false): ChatMessage { + return { id, role, pending, parts: [{ type: 'text', text }] } + } + + it('does NOT stamp a stale assistant zombie with the USER id (the mis-stamp that defeated the sweep)', () => { + // Transcript: [stale assistant zombie from a severed earlier turn, + // fresh user row, fresh completed assistant row]. Frame carries the fresh + // turn's [user_id, assistant_id]. The old top-down role-blind walk gave + // user_id -> zombie and assistant_id -> fresh user row: the zombie wore a + // committed id (unsweepable) and painted as a permanent duplicate. + const stale = textMessage2('assistant-stream-100', 'assistant', 'old severed reply') + const freshUser = textMessage2('user-200-abc', 'user', 'hi') + const freshAssistant = textMessage2('assistant-201', 'assistant', 'hello!') + + const stamped = stampOptimisticTranscriptRows([stale, freshUser, freshAssistant], ['500', '501']) + + // fresh rows get the ids; the stale zombie keeps its optimistic id + expect(stamped.messages.map(row => row.id)).toEqual(['assistant-stream-100', '500', '501']) + expect(stamped.messages[1].role).toBe('user') + expect(stamped.messages[2].role).toBe('assistant') + }) + + it('stale zombie left optimistic is then swept by the poll (end-to-end: no double paint)', () => { + const stale = textMessage2('assistant-stream-100', 'assistant', 'old severed reply') + const freshUser = textMessage2('user-200-abc', 'user', 'hi') + const freshAssistant = textMessage2('assistant-201', 'assistant', 'hello!') + + const stamped = stampOptimisticTranscriptRows([stale, freshUser, freshAssistant], ['500', '501']) + // poll returns the stale turn's committed rows + the fresh ones + const result = appendFetchedMessages(stamped.messages, [ + { id: 400, role: 'assistant', content: 'old severed reply' }, + { id: 500, role: 'user', content: 'hi' }, + { id: 501, role: 'assistant', content: 'hello!' } + ]) + + const texts = result.messages.map(row => row.parts.map(p => (p as { text?: string }).text ?? '').join('')) + expect(texts.filter(t => t === 'old severed reply')).toHaveLength(1) + expect(result.messages.map(row => row.id).sort()).toEqual(['400', '500', '501']) + }) + + it('normal send path unchanged: [user, assistant] stamps its own rows in order', () => { + const stamped = stampOptimisticTranscriptRows( + [textMessage2('user-1-x', 'user', 'q'), { ...textMessage2('assistant-2', 'assistant', 'a'), pending: false }], + ['100', '101'] + ) + expect(stamped.messages.map(row => row.id)).toEqual(['100', '101']) + }) + + it('lone id prefers the assistant streamed row over an older user leftover', () => { + const leftoverUser = textMessage2('user-9-z', 'user', 'unsent leftover') + const freshAssistant = textMessage2('assistant-10', 'assistant', 'reply') + const stamped = stampOptimisticTranscriptRows([leftoverUser, freshAssistant], ['700']) + + expect(stamped.messages.map(row => row.id)).toEqual(['user-9-z', '700']) + }) +}) diff --git a/apps/desktop/src/app/chat/hooks/use-session-changes.ts b/apps/desktop/src/app/chat/hooks/use-session-changes.ts index 63547e3f21ae7..933022cbba66e 100644 --- a/apps/desktop/src/app/chat/hooks/use-session-changes.ts +++ b/apps/desktop/src/app/chat/hooks/use-session-changes.ts @@ -357,28 +357,72 @@ export function stampOptimisticTranscriptRows(messages: readonly ChatMessage[], } } - const stampedIds = new Set() - let nextIdIndex = 0 - const next = messages.map(message => { - if (Number.isInteger(Number(message.id)) || nextIdIndex >= committedIds.length) { - return message + // Stampable = any non-committed-id row: optimistic id (`user-`/`assistant-` + // prefix) OR still-pending. The streamed assistant row is finalized by + // completeAssistantMessage() (pending:false, id `assistant-`) BEFORE + // markTurnComplete() runs this stamp — so a `pending`-only predicate would + // skip it (#352). + const stampable: number[] = [] + messages.forEach((message, index) => { + if (Number.isInteger(Number(message.id))) { + return } + const isOptimisticId = message.id.startsWith('user-') || message.id.startsWith('assistant-') + if (isOptimisticId || message.pending) { + stampable.push(index) + } + }) + + // ROLE-AWARE, TAIL-FIRST assignment. The frame's contract is + // [user_id, assistant_id] for the turn that JUST finished — but the list can + // also hold STALE optimistic rows from an earlier turn whose completion + // frame was severed by a backend restart (this session's live incident, + // 2026-07-15). The old top-down, role-blind walk stamped the USER id onto + // that stale ASSISTANT zombie and the ASSISTANT id onto the real user row — + // the zombie then wore a committed id (invisible to the #361 sweep) and + // painted its stale text as a permanent duplicate, while the current turn's + // rows stayed un-stamped. Assign each id to the LAST unclaimed stampable row + // of the matching role instead (a lone id keeps last-row-any-role), so stale + // zombies stay optimistic — and therefore sweepable. + const assignment = new Map() + const claimed = new Set() + const claim = (id: string, wantRole: 'user' | 'assistant' | null): boolean => { + for (let j = stampable.length - 1; j >= 0; j--) { + const index = stampable[j] + if (claimed.has(index)) { + continue + } + if (wantRole && messages[index].role !== wantRole) { + continue + } + assignment.set(index, id) + claimed.add(index) + return true + } + return false + } + if (committedIds.length === 2) { + claim(committedIds[0], 'user') + claim(committedIds[1], 'assistant') + } else { + // Lone id: the [user, assistant] positional contract can't disambiguate a + // single survivor. Prefer the streamed assistant row (the dup-prone one — + // the poll always re-fetches the final text bubble), fall back to any. + for (const id of committedIds) { + if (!claim(id, 'assistant')) { + claim(id, null) + } + } + } + + const stampedIds = new Set() + const next = messages.map((message, index) => { + const id = assignment.get(index) - // An optimistic row still needs stamping whether or not it is still - // `pending`. The streamed assistant row is finalized by - // completeAssistantMessage() (pending:false, id `assistant-`) BEFORE - // markTurnComplete() runs this stamp — so a `pending`-only predicate skips - // it, it keeps its optimistic id, and the post-completion poll re-appends - // the committed integer row as a DUPLICATE. Recognize any non-committed - // optimistic id (`user-`/`assistant-` prefix) OR a still-pending row. - const isOptimisticId = - message.id.startsWith('user-') || message.id.startsWith('assistant-') - if (!isOptimisticId && !message.pending) { + if (id === undefined) { return message } - const id = committedIds[nextIdIndex] - nextIdIndex += 1 stampedIds.add(id) return { ...message, id, pending: false }