From b2f345f5408a761a11df55fb2c8a7d0f394a1b4a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 16 Aug 2026 04:20:39 -0500 Subject: [PATCH] test(desktop): pin steered-turn transcript order end-to-end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A steered turn's contract — pre-steer output above the correction bubble, post-steer output and the settled reply below it — was fixed across several PRs (#73793/#83151 class, settle fixes) but only covered piecewise: the mid-turn insert as a unit, the settle math as a unit. Nothing drove the real stream reducer through a whole steered turn, and nothing asserted the durable-row hydration renders the same order after reload. Two suites close that: - steer-arrival-order: full event sequences through useMessageStream's real handler + the real optimistic insert — single steer with tool activity, steer racing message.complete, double steer in one turn. - steered-turn-hydration-order: toChatMessages over persisted row shapes copied from a real state.db steered turn, including a tool result that lands after the correction row. --- .../steer-arrival-order.test.tsx | 215 ++++++++++++++++++ .../lib/steered-turn-hydration-order.test.ts | 99 ++++++++ 2 files changed, 314 insertions(+) create mode 100644 apps/desktop/src/app/session/hooks/use-message-stream/steer-arrival-order.test.tsx create mode 100644 apps/desktop/src/lib/steered-turn-hydration-order.test.ts diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/steer-arrival-order.test.tsx b/apps/desktop/src/app/session/hooks/use-message-stream/steer-arrival-order.test.tsx new file mode 100644 index 0000000000..be8fd2edcc --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-message-stream/steer-arrival-order.test.tsx @@ -0,0 +1,215 @@ +// Repro for "when I steer it often sends out of order — a user bubble way +// above" (#73793 / #83151 class). Drives the REAL stream reducer +// (useMessageStream.handleGatewayEvent) and the REAL mid-turn insert +// (appendMidTurnUserMessage — what redirectPrompt's optimistic append uses) +// through a full steered turn, and asserts transcript ORDER: +// +// pre-steer output → steer bubble → post-steer output → settled reply +// +// The steer bubble must land AFTER every assistant row that had already +// streamed when it was typed, and every later delta / tool event / completion +// must land BELOW it — never spliced above, never merged into the sealed +// pre-steer bubble. +import { QueryClient } from '@tanstack/react-query' +import { act, cleanup, render } from '@testing-library/react' +import { useEffect, useRef } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { appendMidTurnUserMessage } from '@/app/session/hooks/use-prompt-actions/rewind' +import type { ClientSessionState } from '@/app/types' +import { chatMessageText } from '@/lib/chat-messages' +import { createClientSessionState } from '@/lib/chat-runtime' +import type { RpcEvent } from '@/types/hermes' + +import { STREAM_DELTA_FLUSH_MS } from './utils' + +import { useMessageStream } from './index' + +const SID = 'steer-order-session' + +let handleEvent: ((event: RpcEvent) => void) | null = null +let states: Map + +function Harness() { + const activeSessionIdRef = useRef(SID) + const sessionStateByRuntimeIdRef = useRef(new Map()) + const queryClientRef = useRef(new QueryClient()) + + const stream = useMessageStream({ + activeSessionIdRef, + hydrateFromStoredSession: vi.fn(async () => undefined), + queryClient: queryClientRef.current, + refreshHermesConfig: vi.fn(async () => undefined), + refreshSessions: vi.fn(async () => undefined), + sessionStateByRuntimeIdRef, + updateSessionState: (sessionId, updater) => { + const current = sessionStateByRuntimeIdRef.current.get(sessionId) ?? createClientSessionState() + const next = updater(current) + sessionStateByRuntimeIdRef.current.set(sessionId, next) + + return next + } + }) + + useEffect(() => { + handleEvent = stream.handleGatewayEvent + states = sessionStateByRuntimeIdRef.current + }, [stream.handleGatewayEvent]) + + return null +} + +async function mountHarness() { + vi.useFakeTimers() + render() + await act(async () => { + await Promise.resolve() + }) +} + +const flushDeltas = async () => { + await act(async () => { + await vi.advanceTimersByTimeAsync(STREAM_DELTA_FLUSH_MS) + }) +} + +const emit = (event: RpcEvent) => act(() => handleEvent?.(event)) + +/** The optimistic steer insert redirectPrompt performs (appendAfterActiveReply). */ +const steer = (text: string) => + act(() => { + const current = states.get(SID) ?? createClientSessionState() + + states.set( + SID, + appendMidTurnUserMessage(current, { + id: `user-${Date.now()}-steer`, + role: 'user', + parts: [{ type: 'text', text }] + }) + ) + }) + +const transcript = () => + (states.get(SID)?.messages ?? []).map(message => `${message.role}:${chatMessageText(message).slice(0, 30)}`) + +describe('steer mid-turn keeps arrival order (user bubble never above prior output)', () => { + beforeEach(() => { + handleEvent = null + states = new Map() + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + vi.restoreAllMocks() + }) + + it('orders pre-steer output → steer → post-steer output → settled reply', async () => { + await mountHarness() + + emit({ payload: {}, session_id: SID, type: 'message.start' }) + emit({ payload: { text: 'first half of the answer' }, session_id: SID, type: 'message.delta' }) + await flushDeltas() + + // Mid-turn tool activity belongs to the pre-steer bubble. + emit({ + payload: { args: { command: 'true' }, name: 'terminal', tool_id: 't1' }, + session_id: SID, + type: 'tool.start' + }) + emit({ payload: { name: 'terminal', result: 'ok', tool_id: 't1' }, session_id: SID, type: 'tool.complete' }) + + steer('actually do it differently') + + // Post-steer deltas must seed a FRESH bubble below the correction. + emit({ payload: { text: 'rebuilt answer after the steer' }, session_id: SID, type: 'message.delta' }) + await flushDeltas() + + const midTurn = states.get(SID)!.messages + const steerIndex = midTurn.findIndex(message => message.role === 'user') + const preSteer = midTurn.slice(0, steerIndex) + const postSteer = midTurn.slice(steerIndex + 1) + + expect(steerIndex, `steer bubble missing: ${transcript().join(' | ')}`).toBeGreaterThan(0) + // Everything the user had already watched arrive stays ABOVE the bubble… + expect(preSteer.some(message => chatMessageText(message).includes('first half'))).toBe(true) + expect(preSteer.every(message => message.role === 'assistant')).toBe(true) + // …sealed (not pending), so no thinking indicator strands above the steer. + expect(preSteer.every(message => message.pending !== true)).toBe(true) + // Post-redirect output continues BELOW the correction, in its own bubble. + expect(postSteer.length).toBeGreaterThan(0) + expect(postSteer.some(message => chatMessageText(message).includes('rebuilt answer'))).toBe(true) + + // Completion settles the post-steer bubble in place — order unchanged. + emit({ + payload: { text: 'rebuilt answer after the steer — done' }, + session_id: SID, + type: 'message.complete' + }) + + const settled = states.get(SID)!.messages + const settledSteerIndex = settled.findIndex(message => message.role === 'user') + const tail = settled.at(-1) + + expect(settledSteerIndex).toBe(steerIndex) + expect(tail?.role).toBe('assistant') + expect(chatMessageText(tail!)).toContain('rebuilt answer after the steer — done') + expect(settled.every(message => message.pending !== true)).toBe(true) + // The final reply is BELOW the steer bubble, not merged into a row above it. + expect(settled.indexOf(tail!)).toBeGreaterThan(settledSteerIndex) + }) + + it('steer with no post-steer deltas: completion settles above, bubble stays at the tail', async () => { + await mountHarness() + + emit({ payload: {}, session_id: SID, type: 'message.start' }) + emit({ payload: { text: 'the whole reply already streamed' }, session_id: SID, type: 'message.delta' }) + await flushDeltas() + + // Steer accepted during the final API call — the reply was already + // complete, so the correction becomes the NEXT turn's prompt. + steer('one more thing') + + emit({ payload: { text: 'the whole reply already streamed' }, session_id: SID, type: 'message.complete' }) + + const messages = states.get(SID)!.messages + const steerIndex = messages.findIndex(message => message.role === 'user') + + // The already-streamed reply settles onto its sealed bubble ABOVE the + // correction; the correction stays the tail, waiting for its own turn — + // no duplicate reply row appended below it. + expect(steerIndex).toBe(messages.length - 1) + expect(messages.filter(message => chatMessageText(message).includes('whole reply already streamed'))).toHaveLength(1) + expect(messages.every(message => message.pending !== true)).toBe(true) + }) + + it('a second steer in the same turn stays below the first (contiguous run, both below prior output)', async () => { + await mountHarness() + + emit({ payload: {}, session_id: SID, type: 'message.start' }) + emit({ payload: { text: 'output before any steer' }, session_id: SID, type: 'message.delta' }) + await flushDeltas() + + steer('first correction') + + emit({ payload: { text: 'output after first steer' }, session_id: SID, type: 'message.delta' }) + await flushDeltas() + + steer('second correction') + + emit({ payload: { text: 'final output' }, session_id: SID, type: 'message.delta' }) + await flushDeltas() + emit({ payload: { text: 'final output' }, session_id: SID, type: 'message.complete' }) + + const roles = states.get(SID)!.messages.map(message => `${message.role}:${chatMessageText(message).slice(0, 24)}`) + + expect(roles, roles.join(' | ')).toEqual([ + 'assistant:output before any steer', + 'user:first correction', + 'assistant:output after first steer', + 'user:second correction', + 'assistant:final output' + ]) + }) +}) diff --git a/apps/desktop/src/lib/steered-turn-hydration-order.test.ts b/apps/desktop/src/lib/steered-turn-hydration-order.test.ts new file mode 100644 index 0000000000..e579544bd7 --- /dev/null +++ b/apps/desktop/src/lib/steered-turn-hydration-order.test.ts @@ -0,0 +1,99 @@ +// Reload parity for a steered turn: the durable rows a redirect persists +// (pre-steer assistant narration + tool rows → interrupted-checkpoint row → +// the user's correction → post-steer continuation with its own tools) must +// hydrate in that same order. A regression here re-creates "my steer shows +// way above the output" on every reload, even when the live path is correct. +// Row shape mirrors real state.db sequences (e.g. ids 731378-731389). +import { describe, expect, it } from 'vitest' + +import type { SessionMessage } from '@/types/hermes' + +import { chatMessageText, toChatMessages } from './chat-messages' + +const steeredTurnRows: SessionMessage[] = [ + { content: 'run the build', role: 'user', timestamp: 100 }, + { + content: 'Kicking off the build:', + role: 'assistant', + timestamp: 101, + tool_calls: [ + { function: { arguments: '{"command":"make"}', name: 'terminal' }, id: 'call-1', type: 'function' } + ] + }, + { content: '{"output":"done","exit_code":0}', role: 'tool', timestamp: 102, tool_call_id: 'call-1', tool_name: 'terminal' }, + { content: 'Build is green. Now writing the docs section', role: 'assistant', timestamp: 103 }, + // Redirect checkpoint: content keeps the visible words; the provider + // scaffolding rides api_content (never shipped to the client). + { content: 'Now the ladder in the resolver:', role: 'assistant', timestamp: 104 }, + { content: 'actually skip docs, fix the tests instead', role: 'user', timestamp: 104.1 }, + { + content: 'Got it — tests first.', + role: 'assistant', + timestamp: 105, + tool_calls: [ + { function: { arguments: '{"command":"pytest"}', name: 'terminal' }, id: 'call-2', type: 'function' } + ] + }, + { content: '{"output":"3 passed","exit_code":0}', role: 'tool', timestamp: 106, tool_call_id: 'call-2', tool_name: 'terminal' }, + { content: 'Tests pass.', role: 'assistant', timestamp: 107 } +] + +describe('toChatMessages on a persisted steered turn', () => { + it('keeps the correction between the output it followed and the output it redirected', () => { + const messages = toChatMessages(steeredTurnRows) + const flat = messages.map(message => `${message.role}:${chatMessageText(message).slice(0, 24)}`) + + const promptIndex = messages.findIndex(m => m.role === 'user' && chatMessageText(m).includes('run the build')) + const correctionIndex = messages.findIndex(m => m.role === 'user' && chatMessageText(m).includes('skip docs')) + + expect(promptIndex, flat.join(' | ')).toBeGreaterThanOrEqual(0) + expect(correctionIndex, flat.join(' | ')).toBeGreaterThan(promptIndex) + + const preCorrection = messages.slice(promptIndex + 1, correctionIndex) + const postCorrection = messages.slice(correctionIndex + 1) + + // Everything streamed before the steer renders above it… + expect(preCorrection.some(m => chatMessageText(m).includes('Build is green'))).toBe(true) + expect(preCorrection.some(m => m.parts.some(p => p.type === 'tool-call' && p.toolCallId === 'call-1'))).toBe(true) + // …and nothing from after the steer leaks above it. + expect(preCorrection.some(m => chatMessageText(m).includes('tests first'))).toBe(false) + expect(preCorrection.some(m => m.parts.some(p => p.type === 'tool-call' && p.toolCallId === 'call-2'))).toBe(false) + + // The redirected continuation (text + its tool call + final reply) is below. + expect(postCorrection.some(m => chatMessageText(m).includes('tests first'))).toBe(true) + expect(postCorrection.some(m => m.parts.some(p => p.type === 'tool-call' && p.toolCallId === 'call-2'))).toBe(true) + expect(chatMessageText(messages.at(-1)!)).toContain('Tests pass') + }) + + it('a tool-result row arriving after the correction cannot pull its call below the user bubble', () => { + // Same turn, but the steer landed between the tool CALL row and its + // RESULT row (accepted at the tool boundary). The result must attach to + // the call above the correction, not materialize a new row below it. + const rows: SessionMessage[] = [ + { content: 'go', role: 'user', timestamp: 100 }, + { + content: '', + role: 'assistant', + timestamp: 101, + tool_calls: [ + { function: { arguments: '{"command":"sleep 60"}', name: 'terminal' }, id: 'call-9', type: 'function' } + ] + }, + { content: 'never mind, stop that', role: 'user', timestamp: 101.5 }, + { content: '{"output":"interrupted"}', role: 'tool', timestamp: 102, tool_call_id: 'call-9', tool_name: 'terminal' }, + { content: 'Stopped.', role: 'assistant', timestamp: 103 } + ] + + const messages = toChatMessages(rows) + const correctionIndex = messages.findIndex(m => m.role === 'user' && chatMessageText(m).includes('never mind')) + const callRowIndex = messages.findIndex(m => m.parts.some(p => p.type === 'tool-call' && p.toolCallId === 'call-9')) + + expect(correctionIndex).toBeGreaterThan(0) + expect(callRowIndex).toBeGreaterThanOrEqual(0) + expect(callRowIndex, messages.map(m => m.role).join(' | ')).toBeLessThan(correctionIndex) + // Exactly one rendering of the call — the late result must not clone it. + expect( + messages.flatMap(m => m.parts).filter(p => p.type === 'tool-call' && p.toolCallId === 'call-9') + ).toHaveLength(1) + }) +})