From c4cdb3ae38754a0b7cec98e2dcb66c223c56f46a Mon Sep 17 00:00:00 2001 From: Intron Forge <121190911+ColdSlither@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:40:43 -0400 Subject: [PATCH] fix(desktop): do not count backend-authored notices as transcript staleness (#123574) A model switch persists display_kind=model_switch with role=user (tui_gateway/server.py). Hydration renders that row as a system message ("model changed"), so the authoritative latest page holds one more message than the window looking at the same chat. The stale-transcript guard measured staleness as `remoteChat.length > localMessages.length`, so a session that had switched models reported "This window was behind another view of the same chat", refused the send, and repeated the refusal on every retry. No second window existed, and nothing in the session was damaged. Measure authored content instead of array length: * hydration.ts marks a converted backend notice with ChatMessage.systemNotice, via one NOTICE_DISPLAY_KINDS predicate that now also drives the existing system-role decision. * stale-transcript-guard.ts compares authoredMessageCount on both sides, so a notice never counts as another view's work. * use-session-actions/utils.ts classifies the new field in IGNORED_FIELDS: the transcript paints the row from role + parts, and role is already COMPARED. Notices still render unchanged. Tool rows folded into an assistant bubble keep the existing behavior. A genuinely forked chat is still refused, covered by tests. Trade-off: a difference consisting only of notices no longer installs the page, so a "model changed" row can wait for the next natural hydrate. The send proceeds, which is the point of the guard. Tests: new apps/desktop/src/lib/stale-transcript-guard.test.ts, 5 cases (the notice regression, both real-fork cases, the identical-page case, and the empty-page contracts). The guard had no test before. Verified with `vitest run --project ui` (8944 passed; the 2 failures in voice-prefs.test.ts are pre-existing and reproduce with these edits stashed) and `npm run typecheck` (clean). --- .../hooks/use-session-actions/utils.ts | 6 +- .../src/lib/chat-messages/hydration.ts | 31 ++++--- apps/desktop/src/lib/chat-messages/types.ts | 7 ++ .../src/lib/stale-transcript-guard.test.ts | 89 +++++++++++++++++++ .../desktop/src/lib/stale-transcript-guard.ts | 26 ++++-- 5 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/src/lib/stale-transcript-guard.test.ts diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index 4413467c88..4e66433c1a 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -196,6 +196,10 @@ function preserveStructuralParts(message: ChatMessage, previous: ChatMessage): C // attachmentRefs — composer-side metadata; already reconciled in reconcileResumeMessages // serverRowSpan — backend rows the folded message covers; the older-page offset // accounting reads it, the transcript never paints it +// systemNotice — hydration's provenance flag for a backend-authored notice +// (a model switch, a process completion); the stale-transcript +// compare reads it, while the visible system row is painted +// from role + parts, and role is already COMPARED // // If your new field affects what the user sees in the transcript, add it to // COMPARED. If it's metadata that shouldn't trigger a re-render, add it to @@ -229,7 +233,7 @@ const COMPARED_FIELDS = [ 'durationS' ] as const -const IGNORED_FIELDS = ['attachmentRefs', 'parts', 'serverRowSpan'] as const +const IGNORED_FIELDS = ['attachmentRefs', 'parts', 'serverRowSpan', 'systemNotice'] as const // Compile-time check: every ChatMessagePart discriminant must be handled by // chatPartsEquivalent. If @assistant-ui adds a new part type, this fails tsc. diff --git a/apps/desktop/src/lib/chat-messages/hydration.ts b/apps/desktop/src/lib/chat-messages/hydration.ts index 31e886a2ca..68b0136ee8 100644 --- a/apps/desktop/src/lib/chat-messages/hydration.ts +++ b/apps/desktop/src/lib/chat-messages/hydration.ts @@ -140,6 +140,25 @@ function transcriptContent(displayKind: SessionMessage['display_kind'], content: return displayKind === 'hidden' ? null : content } +/** + * Backend-authored transcript notices. The gateway persists these itself and no + * view "sent" them, so they render as system rows but are not authored + * transcript content (see `ChatMessage.systemNotice`). + */ +const NOTICE_DISPLAY_KINDS = [ + 'model_switch', + 'async_delegation_complete', + 'process_complete', + 'auto_continue', + 'personality_switch', + // Hermes closing a failed turn, not the model speaking. + 'failed_turn' +] as const + +function isMachineNotice(displayKind: SessionMessage['display_kind']): boolean { + return displayKind !== undefined && (NOTICE_DISPLAY_KINDS as readonly string[]).includes(displayKind) +} + // A remote backend older than this app serves display_metadata as raw JSON text, // and `in` throws on a primitive — which used to fail the whole session resume. function parseDisplayMetadata(metadata: SessionMessage['display_metadata']): null | Record { @@ -348,16 +367,7 @@ export function toChatMessages(messages: SessionMessage[]): ChatMessage[] { timelineDisplayContent(message, displayContentForMessage(message.role, content)) ) - const displayRole = - message.display_kind === 'model_switch' || - message.display_kind === 'async_delegation_complete' || - message.display_kind === 'process_complete' || - message.display_kind === 'auto_continue' || - message.display_kind === 'personality_switch' || - // Hermes closing a failed turn, not the model speaking. - message.display_kind === 'failed_turn' - ? 'system' - : message.role + const displayRole = isMachineNotice(message.display_kind) ? 'system' : message.role // Persisted user turns carry `@image:` directive lines inline in // the text (see tui_gateway/server.py's persist-time rewrite). The @@ -496,6 +506,7 @@ export function toChatMessages(messages: SessionMessage[]): ChatMessage[] { ? { asyncResult: asyncResultBody(displayContentForMessage(message.role, message.content || content)) } : {}), ...(message.display_kind === 'process_complete' ? { asyncResultKind: 'process' as const } : {}), + ...(isMachineNotice(message.display_kind) ? { systemNotice: true } : {}), timestamp: earliestTimestamp(message.timestamp, ...parts.map(part => part.timestamp)), ...(rowId !== undefined ? { rowId } : {}), ...(pendingAbsorbedRows > 0 ? { serverRowSpan: pendingAbsorbedRows + 1 } : {}), diff --git a/apps/desktop/src/lib/chat-messages/types.ts b/apps/desktop/src/lib/chat-messages/types.ts index 082480ff3b..970bb04195 100644 --- a/apps/desktop/src/lib/chat-messages/types.ts +++ b/apps/desktop/src/lib/chat-messages/types.ts @@ -66,6 +66,13 @@ export type ChatMessage = { serverRowSpan?: number /** Emoji reactions on this message — one per author (see MessageReaction). */ reactions?: MessageReaction[] + /** Backend-authored transcript notice rather than a message any view sent: a + * model switch, an auto-continue, a background-process completion. It renders + * on the timeline like any other system row but belongs to no view, so the + * stale-transcript compare must not count it (see + * `messagesIfTranscriptBehind`) — counting it made one model switch report a + * second window ahead and refuse every send. */ + systemNotice?: boolean } export type GatewayEventPayload = { diff --git a/apps/desktop/src/lib/stale-transcript-guard.test.ts b/apps/desktop/src/lib/stale-transcript-guard.test.ts new file mode 100644 index 0000000000..89c6563ee6 --- /dev/null +++ b/apps/desktop/src/lib/stale-transcript-guard.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest' + +import type { SessionMessage } from '@/types/hermes' + +import { toChatMessages } from './chat-messages' +import { messagesIfTranscriptBehind } from './stale-transcript-guard' + +/** + * The guard refuses a send when the authoritative latest page holds MORE + * transcript than the window does, rather than forking the session (#65047). + * It measures that difference with `remoteChat.length > localMessages.length` + * after `toChatMessages`. + * + * A backend-authored NOTICE also arrives as a `ChatMessage` (`model changed`, + * `background agent work finished`): `tui_gateway/server.py` persists + * `display_kind=model_switch` with `role=user` on an in-place model switch, and + * hydration renders it as a system row. Nothing about it means another window + * exists — but it inflates the page by one message per event, so the guard + * reported "This window was behind another view of the same chat" to a user who + * had only switched models, refused the send, and said the same thing on every + * retry. Staleness must be measured in AUTHORED content, not array length. + */ + +const row = (over: Partial & Pick): SessionMessage => ({ + content: '', + timestamp: 1_700_000_000, + ...over +}) + +const userTurn = (id: number, text: string): SessionMessage => + row({ content: text, id, role: 'user', timestamp: 1_700_000_000 + id }) + +const assistantTurn = (id: number, text: string): SessionMessage => + row({ content: text, id, role: 'assistant', timestamp: 1_700_000_000 + id }) + +/** What an in-place model switch persists, exactly as the gateway writes it. */ +const modelSwitchNotice = (id: number): SessionMessage => + row({ content: 'switched to another model', display_kind: 'model_switch', id, role: 'user', timestamp: 1_700_000_000 + id }) + +describe('messagesIfTranscriptBehind', () => { + it('does not treat a backend-authored notice as another view being ahead', () => { + const windowMessages = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer')]) + const page = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer'), modelSwitchNotice(3)]) + + // The notice is a real message in the page: this is what skews a length compare. + expect(page).toHaveLength(3) + expect(page.map(message => message.role)).toEqual(['user', 'assistant', 'system']) + + expect(messagesIfTranscriptBehind(windowMessages, page)).toBeNull() + }) + + it('still refuses when another view advanced the chat with a real turn', () => { + const windowMessages = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer')]) + + const page = toChatMessages([ + userTurn(1, 'ask'), + assistantTurn(2, 'answer'), + userTurn(4, 'sent from another window') + ]) + + expect(messagesIfTranscriptBehind(windowMessages, page)).not.toBeNull() + }) + + it('still refuses when a notice arrives alongside a reply this window never saw', () => { + const windowMessages = toChatMessages([userTurn(1, 'ask'), assistantTurn(2, 'answer')]) + + const page = toChatMessages([ + userTurn(1, 'ask'), + assistantTurn(2, 'answer'), + modelSwitchNotice(3), + assistantTurn(4, 'a reply this window never rendered') + ]) + + expect(messagesIfTranscriptBehind(windowMessages, page)).not.toBeNull() + }) + + it('is current when both sides hold the same notices and the same turns', () => { + const rows = [userTurn(1, 'ask'), assistantTurn(2, 'answer'), modelSwitchNotice(3)] + + expect(messagesIfTranscriptBehind(toChatMessages(rows), toChatMessages(rows))).toBeNull() + }) + + it('is current when the page is empty, and refreshes a window that holds nothing', () => { + const rows = [userTurn(1, 'ask'), assistantTurn(2, 'answer')] + + expect(messagesIfTranscriptBehind(toChatMessages(rows), [])).toBeNull() + expect(messagesIfTranscriptBehind([], toChatMessages(rows))).toEqual(toChatMessages(rows)) + }) +}) diff --git a/apps/desktop/src/lib/stale-transcript-guard.ts b/apps/desktop/src/lib/stale-transcript-guard.ts index b6ac5c9f7e..9c7faccbba 100644 --- a/apps/desktop/src/lib/stale-transcript-guard.ts +++ b/apps/desktop/src/lib/stale-transcript-guard.ts @@ -20,15 +20,26 @@ export function profileScopeForSessionOwner(owner: SessionOwnerScope): ProfileSc } } +/** + * Transcript content a view actually authored. Backend-written notices + * (`ChatMessage.systemNotice`) render on the timeline but belong to no view, so + * counting them reports a second window that does not exist: an in-place model + * switch alone refused every send with "this window was behind another view of + * the same chat". + */ +function authoredMessageCount(messages: ChatMessage[]): number { + return messages.reduce((count, message) => (message.systemNotice ? count : count + 1), 0) +} + /** * Chat messages to install when the authoritative latest page is ahead of the * local view. Null when the local view is current. * - * Length is compared after `toChatMessages`, so tool rows folded into an - * assistant bubble are not "ahead". A backfilled prefix is kept when the - * refreshed tail anchors inside it. Live stream ids that do not anchor still - * use length, so the window that just finished the turn is not blocked when - * the counts match. + * Authored content is compared after `toChatMessages`, so tool rows folded into + * an assistant bubble are not "ahead", and neither is a backend-authored + * notice. A backfilled prefix is kept when the refreshed tail anchors inside + * it. Live stream ids that do not anchor still use the count, so the window + * that just finished the turn is not blocked when the counts match. */ export function messagesIfTranscriptBehind( localMessages: ChatMessage[], @@ -43,12 +54,13 @@ export function messagesIfTranscriptBehind( } const grafted = graftRefreshedTailOntoBackfill(remoteChat, localMessages) + const localAuthored = authoredMessageCount(localMessages) if (grafted === remoteChat) { - return remoteChat.length > localMessages.length ? remoteChat : null + return authoredMessageCount(remoteChat) > localAuthored ? remoteChat : null } - return grafted.length > localMessages.length ? grafted : null + return authoredMessageCount(grafted) > localAuthored ? grafted : null } /**