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 b82908a3ee7d9..6039ef12047da 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 @@ -645,3 +645,95 @@ describe('reconnect-seam zombie optimistic rows (severed message.complete stamp) expect(committed.messages.map(row => row.id)).toEqual(['700']) }) }) + +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']) + }) +}) + +describe('Greptile #364 edge frames', () => { + function tm(id: string, role: ChatMessage['role'], text: string, pending = false): ChatMessage { + return { id, role, pending, parts: [{ type: 'text', text }] } + } + + it('no user-role stampable row: user id is deliberately NOT cross-role stamped', () => { + // User row already committed; only the streamed assistant row + a stale + // assistant zombie remain. The user id must NOT land on either assistant + // row (cross-role mis-stamp); the assistant id lands on the FRESH tail row. + const zombie = tm('assistant-stream-old', 'assistant', 'stale') + const freshAssistant = tm('assistant-2', 'assistant', 'reply') + const stamped = stampOptimisticTranscriptRows([zombie, freshAssistant], ['500', '501']) + + expect(stamped.messages.map(row => row.id)).toEqual(['assistant-stream-old', '501']) + expect(stamped.stampedIds.has('500')).toBe(false) // user id dropped, poll reconciles + }) + + it('3+ ids (array + scalar fields both populated): positional pair stays role-aware, extras unstamped', () => { + const user = tm('user-1-a', 'user', 'q') + const assistant = tm('assistant-2', 'assistant', 'a') + const stamped = stampOptimisticTranscriptRows([user, assistant], ['500', '501', '502']) + + // Greptile #398: the first two ids keep the [user_id, assistant_id] + // positional convention — 500 lands on the USER row, 501 on the assistant + // row (an assistant-preferring walk over all ids reversed them). The third + // id has no unclaimed row left and is left for the poll. + expect(stamped.messages.map(row => row.id)).toEqual(['500', '501']) + expect(stamped.messages[0].role).toBe('user') + expect(stamped.messages[1].role).toBe('assistant') + expect(stamped.stampedIds.size).toBe(2) + expect(stamped.stampedIds.has('502')).toBe(false) + }) +}) 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 4ce6875d85059..f9ca36ee46257 100644 --- a/apps/desktop/src/app/chat/hooks/use-session-changes.ts +++ b/apps/desktop/src/app/chat/hooks/use-session-changes.ts @@ -359,30 +359,96 @@ export function stampOptimisticTranscriptRows(messages: readonly ChatMessage[], } } - const stampedIds = new Set() - let nextIdIndex = 0 + // 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) + } + }) - const next = messages.map(message => { - if (Number.isInteger(Number(message.id)) || nextIdIndex >= committedIds.length) { - return message + // 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) { + // [user_id, assistant_id] positional contract. A failed role-scoped claim + // deliberately DROPS the id (Greptile #364 flagged the silent discard — + // it is intentional, not a bug): if no user-role stampable row exists, + // the user row already committed, so the poll reconciles it by id with + // no optimistic twin to duplicate. Falling back to any-role here would + // stamp the USER id onto an assistant row — with a stale assistant + // zombie present, that re-creates the exact cross-role mis-stamp this + // function exists to prevent (zombie wears a committed id, unsweepable). + // Unstamped-but-committed is safe; cross-role-stamped is not. + claim(committedIds[0], 'user') + claim(committedIds[1], 'assistant') + } else if (committedIds.length > 2) { + // 3+ ids (array + scalar fields both populated — Greptile #364). The first + // two ids still follow the [user_id, assistant_id] positional convention, + // so claim them role-aware exactly like the 2-id case (Greptile #398: + // assistant-preferring EVERY id here put committedIds[0] — the user's id — + // onto an assistant row, a cross-role reversal). Only the EXTRAS beyond the + // positional pair fall to assistant-then-any; ids beyond the stampable rows + // are simply not stamped (the poll reconciles them). + claim(committedIds[0], 'user') + claim(committedIds[1], 'assistant') + for (const id of committedIds.slice(2)) { + if (!claim(id, 'assistant')) { + claim(id, null) + } + } + } else { + // Lone id: the 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 unclaimed row. + for (const id of committedIds) { + if (!claim(id, 'assistant')) { + claim(id, null) + } + } + } - // 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-') + const stampedIds = new Set() + const next = messages.map((message, index) => { + const id = assignment.get(index) - if (!isOptimisticId && !message.pending) { + if (id === undefined) { return message } - const id = committedIds[nextIdIndex] - nextIdIndex += 1 stampedIds.add(id) return { ...message, id, pending: false }