fix(desktop): persist inline assistant errors across hydrate/resume
- Detect provider failure text arriving via message.complete (HTTP 4xx, "API call failed after N retries", Provider/Gateway error: ...) and persist as an inline assistant error instead of regular completion text, blocking the hydrate that was wiping it. - preserveLocalAssistantErrors: merge by id so same-id hydrated messages keep their local error, and preserve the optimistic user+error pair as a unit (with tail-user dedupe). - Hook all hydrate/resume writers (use-session-actions resume + fallback, hydrateFromStoredSession, syncSessionStateToView) into the merge so stale snapshots can't clobber a failed turn. - Add error to chatMessagesEquivalent so the resume diff actually sees error-only changes and paints them. - editMessage on a failed turn now submits a plain resend (no truncate_before_user_ordinal) and retries plainly on the "no longer in session history" race. Style polish on touched files: - Inline error: text-only treatment (no card). - User stop / edit-composer send: shared Tabler IconPlayerStopFilled glyph + shared icon-button class slot for parity.
This commit is contained in:
Brooklyn Nicholson
@@ -1,9 +1,10 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { ChatMessagePart } from './chat-messages'
|
||||
import type { ChatMessage, ChatMessagePart } from './chat-messages'
|
||||
import {
|
||||
appendAssistantTextPart,
|
||||
chatMessageText,
|
||||
preserveLocalAssistantErrors,
|
||||
renderMediaTags,
|
||||
toChatMessages,
|
||||
upsertToolPart
|
||||
@@ -142,6 +143,173 @@ describe('renderMediaTags', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('preserveLocalAssistantErrors', () => {
|
||||
it('preserves a local user+error pair when hydration omits the failed turn', () => {
|
||||
const nextMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'stored-user',
|
||||
parts: [{ text: 'earlier', type: 'text' }],
|
||||
role: 'user'
|
||||
}
|
||||
]
|
||||
|
||||
const currentMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'stored-user',
|
||||
parts: [{ text: 'earlier', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
id: 'user-123',
|
||||
parts: [{ text: 'new prompt', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
error: 'OpenRouter 403',
|
||||
id: 'assistant-error-1',
|
||||
parts: [],
|
||||
role: 'assistant'
|
||||
}
|
||||
]
|
||||
|
||||
const merged = preserveLocalAssistantErrors(nextMessages, currentMessages)
|
||||
|
||||
expect(merged.map(message => message.id)).toEqual(['stored-user', 'user-123', 'assistant-error-1'])
|
||||
expect(merged[2]?.error).toBe('OpenRouter 403')
|
||||
})
|
||||
|
||||
it('does not keep orphan local user turns when there is no inline assistant error', () => {
|
||||
const nextMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'stored-user',
|
||||
parts: [{ text: 'earlier', type: 'text' }],
|
||||
role: 'user'
|
||||
}
|
||||
]
|
||||
|
||||
const currentMessages: ChatMessage[] = [
|
||||
...nextMessages,
|
||||
{
|
||||
id: 'user-123',
|
||||
parts: [{ text: 'new prompt', type: 'text' }],
|
||||
role: 'user'
|
||||
}
|
||||
]
|
||||
|
||||
const merged = preserveLocalAssistantErrors(nextMessages, currentMessages)
|
||||
|
||||
expect(merged.map(message => message.id)).toEqual(['stored-user'])
|
||||
})
|
||||
|
||||
it('does not duplicate local user when stored history already has equivalent text', () => {
|
||||
const nextMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'stored-user',
|
||||
parts: [{ text: 'hi', type: 'text' }],
|
||||
role: 'user'
|
||||
}
|
||||
]
|
||||
|
||||
const currentMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'optimistic-user',
|
||||
parts: [{ text: 'hi', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
error: 'OpenRouter 403',
|
||||
id: 'assistant-error-1',
|
||||
parts: [],
|
||||
role: 'assistant'
|
||||
}
|
||||
]
|
||||
|
||||
const merged = preserveLocalAssistantErrors(nextMessages, currentMessages)
|
||||
|
||||
expect(merged.map(message => message.id)).toEqual(['stored-user', 'assistant-error-1'])
|
||||
})
|
||||
|
||||
it('keeps local user when only older history has equivalent text', () => {
|
||||
const nextMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'older-user',
|
||||
parts: [{ text: 'hi', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
id: 'older-assistant',
|
||||
parts: [{ text: 'hello', type: 'text' }],
|
||||
role: 'assistant'
|
||||
},
|
||||
{
|
||||
id: 'tail-user',
|
||||
parts: [{ text: 'different prompt', type: 'text' }],
|
||||
role: 'user'
|
||||
}
|
||||
]
|
||||
|
||||
const currentMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'optimistic-user',
|
||||
parts: [{ text: 'hi', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
error: 'OpenRouter 403',
|
||||
id: 'assistant-error-1',
|
||||
parts: [],
|
||||
role: 'assistant'
|
||||
}
|
||||
]
|
||||
|
||||
const merged = preserveLocalAssistantErrors(nextMessages, currentMessages)
|
||||
|
||||
expect(merged.map(message => message.id)).toEqual([
|
||||
'older-user',
|
||||
'older-assistant',
|
||||
'tail-user',
|
||||
'optimistic-user',
|
||||
'assistant-error-1'
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps local assistant error when hydrated message reuses same id', () => {
|
||||
const nextMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'user-1',
|
||||
parts: [{ text: 'new prompt', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
id: 'assistant-stream-1',
|
||||
parts: [{ text: '', type: 'text' }],
|
||||
role: 'assistant'
|
||||
}
|
||||
]
|
||||
|
||||
const currentMessages: ChatMessage[] = [
|
||||
{
|
||||
id: 'user-1',
|
||||
parts: [{ text: 'new prompt', type: 'text' }],
|
||||
role: 'user'
|
||||
},
|
||||
{
|
||||
error: 'OpenRouter 403',
|
||||
id: 'assistant-stream-1',
|
||||
parts: [],
|
||||
role: 'assistant'
|
||||
}
|
||||
]
|
||||
|
||||
const merged = preserveLocalAssistantErrors(nextMessages, currentMessages)
|
||||
|
||||
const assistant = merged.find(message => message.id === 'assistant-stream-1')
|
||||
|
||||
expect(assistant?.error).toBe('OpenRouter 403')
|
||||
expect(assistant?.pending).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('upsertToolPart', () => {
|
||||
it('preserves inline diffs from tool completion events', () => {
|
||||
const parts = upsertToolPart(
|
||||
@@ -221,6 +389,7 @@ describe('upsertToolPart', () => {
|
||||
|
||||
const completedResult =
|
||||
completed[0] && 'result' in completed[0] ? (completed[0].result as Record<string, unknown>) : {}
|
||||
|
||||
const clearedResult = cleared[0] && 'result' in cleared[0] ? (cleared[0].result as Record<string, unknown>) : {}
|
||||
|
||||
expect(completedResult.todos).toEqual([{ content: 'Boil water', id: 'boil', status: 'in_progress' }])
|
||||
|
||||
@@ -12,6 +12,7 @@ export type ChatMessage = {
|
||||
parts: ChatMessagePart[]
|
||||
timestamp?: number
|
||||
pending?: boolean
|
||||
error?: string
|
||||
branchGroupId?: string
|
||||
hidden?: boolean
|
||||
/** Composer attachment ref strings (`@file:...`, `@image:...`) sent with this user message. */
|
||||
@@ -801,6 +802,77 @@ export function toChatMessages(messages: SessionMessage[]): ChatMessage[] {
|
||||
)
|
||||
}
|
||||
|
||||
export function preserveLocalAssistantErrors(
|
||||
nextMessages: ChatMessage[],
|
||||
currentMessages: ChatMessage[]
|
||||
): ChatMessage[] {
|
||||
const localById = new Map(currentMessages.map(message => [message.id, message]))
|
||||
|
||||
const mergedNextMessages = nextMessages.map(message => {
|
||||
if (message.role !== 'assistant' || message.error || message.hidden) {
|
||||
return message
|
||||
}
|
||||
|
||||
const local = localById.get(message.id)
|
||||
|
||||
if (!local || local.role !== 'assistant' || !local.error || local.hidden) {
|
||||
return message
|
||||
}
|
||||
|
||||
return {
|
||||
...message,
|
||||
error: local.error,
|
||||
pending: false
|
||||
}
|
||||
})
|
||||
|
||||
const existingIds = new Set(mergedNextMessages.map(message => message.id))
|
||||
const preserveIds = new Set<string>()
|
||||
const normalize = (value: string) => value.replace(/\s+/g, ' ').trim()
|
||||
const tailUserInNext = [...mergedNextMessages].reverse().find(message => message.role === 'user' && !message.hidden)
|
||||
const tailUserText = tailUserInNext ? normalize(chatMessageText(tailUserInNext)) : ''
|
||||
const tailUserRefs = tailUserInNext ? (tailUserInNext.attachmentRefs ?? []).join('\n') : ''
|
||||
|
||||
const matchesTailUserInNext = (candidate: ChatMessage) =>
|
||||
Boolean(tailUserInNext) &&
|
||||
normalize(chatMessageText(candidate)) === tailUserText &&
|
||||
(candidate.attachmentRefs ?? []).join('\n') === tailUserRefs
|
||||
|
||||
for (let index = 0; index < currentMessages.length; index += 1) {
|
||||
const message = currentMessages[index]
|
||||
|
||||
if (message.role !== 'assistant' || !message.error || message.hidden || existingIds.has(message.id)) {
|
||||
continue
|
||||
}
|
||||
|
||||
preserveIds.add(message.id)
|
||||
|
||||
for (let probe = index - 1; probe >= 0; probe -= 1) {
|
||||
const candidate = currentMessages[probe]
|
||||
|
||||
if (candidate.hidden) {
|
||||
continue
|
||||
}
|
||||
|
||||
if (candidate.role === 'user' && !existingIds.has(candidate.id) && !matchesTailUserInNext(candidate)) {
|
||||
preserveIds.add(candidate.id)
|
||||
}
|
||||
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if (preserveIds.size === 0) {
|
||||
return mergedNextMessages
|
||||
}
|
||||
|
||||
const preserved = currentMessages
|
||||
.filter(message => preserveIds.has(message.id))
|
||||
.map(message => ({ ...message, pending: false }))
|
||||
|
||||
return [...mergedNextMessages, ...preserved]
|
||||
}
|
||||
|
||||
export function branchGroupForUser(userMessage: ChatMessage): string {
|
||||
return `branch:${userMessage.id}`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user