Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions apps/desktop/src/app/chat/hooks/use-session-changes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
})
})
78 changes: 61 additions & 17 deletions apps/desktop/src/app/chat/hooks/use-session-changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,28 +357,72 @@ export function stampOptimisticTranscriptRows(messages: readonly ChatMessage[],
}
}

const stampedIds = new Set<string>()
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-<ts>`) 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<number, string>()
const claimed = new Set<number>()
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')
Comment on lines +404 to +406

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent drop when no user-role stampable row exists

In the two-id branch, claim(committedIds[0], 'user') returns false when there are no unclaimed user-role rows in stampable — and the return value is silently discarded. Unlike the lone-id path (which falls back to claim(id, null)), the user ID is simply lost. This is fine if the user row was already committed before this stamp fires, but it could silently drop a valid user ID in any edge case where the optimistic user row was cleared without getting a committed integer ID. Adding a fallback claim(committedIds[0], null) when the role-specific claim fails would mirror the lone-id path's defensive behaviour.

} 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)
}
}
}
Comment on lines +404 to +416

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The else branch handles every committedIds.length !== 2 case — including lengths of 3 or more — but its logic and comment only make sense for exactly one ID. extractCommittedMessageIds can return 3+ IDs when both an array field (message_ids) and a scalar field (message_id) are non-empty in the same payload. In that situation the for loop calls claim(id, 'assistant') for every ID, including committedIds[0] which is the positional user ID — repeating the exact cross-role mis-stamp this PR fixes, just for the >2 case. Tightening the condition to === 1 keeps the assistant-preference logic scoped to its documented intent.

Suggested change
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)
}
}
}
if (committedIds.length === 2) {
claim(committedIds[0], 'user')
claim(committedIds[1], 'assistant')
} else if (committedIds.length === 1) {
// 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.
if (!claim(committedIds[0], 'assistant')) {
claim(committedIds[0], null)
}
} else {
// Unexpected count (>2 or 0 after the early-exit guard): fall back to
// role-blind tail-first assignment so at least the tail rows get stamped.
for (const id of committedIds) {
claim(id, null)
}
}


const stampedIds = new Set<string>()
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-<ts>`) 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 }
Expand Down
Loading