diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts index 6fddaead..3e86d79a 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts @@ -51,6 +51,7 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp const { messages, + historicalCount, streamingText, isConnected, isGenerating, @@ -464,6 +465,7 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp return { messages, + historicalCount, streamingText, isConnected, isGenerating, diff --git a/src/workspaces/officerdev/src/apps/Chat/components/MessageList.tsx b/src/workspaces/officerdev/src/apps/Chat/components/MessageList.tsx index 2535633d..a2c14d00 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/MessageList.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/MessageList.tsx @@ -18,6 +18,7 @@ type MessageListProps = { export const MessageList = ({ manager }: MessageListProps) => { const { messages, + historicalCount, streamingText, isGenerating, showJumpToBottom, @@ -29,8 +30,11 @@ export const MessageList = ({ manager }: MessageListProps) => { stickToBottom, } = manager; - // The live turn stays open; everything before it is folded. See `turn-rows.ts`. - const rows = useMemo(() => buildRows(messages, isGenerating), [messages, isGenerating]); + // Turns from this sitting stay open; anything loaded from the server folds. See `turn-rows.ts`. + const rows = useMemo( + () => buildRows(messages, isGenerating, historicalCount), + [messages, isGenerating, historicalCount], + ); // Which folds you have opened. Lifted out of the fold itself because rows are virtualised: state held // inside one would be thrown away the moment it scrolled past the overscan window and silently snap shut. diff --git a/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.test.ts b/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.test.ts index 0b81f471..b5d6b1df 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.test.ts +++ b/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.test.ts @@ -13,9 +13,14 @@ const tool = (id: string, isError?: boolean): ChatMessage => ({ ...(isError ? { isError: true } : {}), }); -/** The shape under test is the row sequence, not the keys. */ -const shape = (messages: ChatMessage[], isGenerating = false) => - buildRows(messages, isGenerating).map((r) => (r.kind === 'message' ? r.message.role : r.kind)); +/** + * The shape under test is the row sequence, not the keys. + * + * `historicalCount` defaults to the whole list — a reload, where everything came from the server. That is + * the case most of these describe; the live case sets it to 0 explicitly. + */ +const shape = (messages: ChatMessage[], isGenerating = false, historicalCount = messages.length) => + buildRows(messages, isGenerating, historicalCount).map((r) => (r.kind === 'message' ? r.message.role : r.kind)); describe('buildRows', () => { test('the live turn is untouched — every row stays where it was', () => { @@ -34,13 +39,13 @@ describe('buildRows', () => { test('the fold counts what it hid, failures included', () => { const messages = [user('first'), say('note'), tool('a'), tool('b', true), say('answer'), user('second')]; - const fold = buildRows(messages, false).find((r) => r.kind === 'fold'); + const fold = buildRows(messages, false, messages.length).find((r) => r.kind === 'fold'); expect(fold?.kind === 'fold' && fold.counts).toEqual({ tools: 2, notes: 1, tasks: 0, failed: 1 }); }); test('a fold opens onto exactly the messages it replaced, in order', () => { const messages = [user('first'), say('note'), tool('a'), say('answer'), user('second')]; - const fold = buildRows(messages, false).find((r) => r.kind === 'fold'); + const fold = buildRows(messages, false, messages.length).find((r) => r.kind === 'fold'); expect(fold?.kind === 'fold' && fold.messages).toEqual([say('note'), tool('a')]); }); @@ -67,8 +72,10 @@ describe('buildRows', () => { expect(shape(messages)).toEqual(['user', 'fold', 'compact', 'assistant', 'user']); }); + // The marker tests describe a turn from this sitting, so they pass 0: whether a finished turn also + // folds is a separate question, pinned in its own tests below. test('a turn that ends on a tool call is marked finished rather than left hanging', () => { - expect(shape([user('go'), tool('a')], false)).toEqual(['user', 'tool', 'turn-end']); + expect(shape([user('go'), tool('a')], false, 0)).toEqual(['user', 'tool', 'turn-end']); }); test('a turn still running is not marked finished', () => { @@ -76,11 +83,15 @@ describe('buildRows', () => { }); test('a turn that failed keeps its own row and gets no marker', () => { - expect(shape([user('go'), tool('a'), { role: 'error', text: 'boom' }], false)).toEqual(['user', 'tool', 'error']); + expect(shape([user('go'), tool('a'), { role: 'error', text: 'boom' }], false, 0)).toEqual([ + 'user', + 'tool', + 'error', + ]); }); test('an interrupted turn gets no marker either', () => { - expect(shape([user('go'), tool('a'), { role: 'interrupted' }], false)).toEqual(['user', 'tool', 'interrupted']); + expect(shape([user('go'), tool('a'), { role: 'interrupted' }], false, 0)).toEqual(['user', 'tool', 'interrupted']); }); test('a cost row alone is not an answer — the marker still appears, above it', () => { @@ -89,20 +100,62 @@ describe('buildRows', () => { tool('a'), { role: 'result', cost: { inputTokens: 1, outputTokens: 1, totalUSD: 0 } }, ]; - expect(shape(messages, false)).toEqual(['user', 'tool', 'turn-end', 'result']); + expect(shape(messages, false, 0)).toEqual(['user', 'tool', 'turn-end', 'result']); }); test('an empty assistant block does not count as having answered', () => { - expect(shape([user('go'), tool('a'), say(' ')], false)).toEqual(['user', 'tool', 'turn-end', 'assistant']); + expect(shape([user('go'), tool('a'), say(' ')], false, 0)).toEqual(['user', 'tool', 'turn-end', 'assistant']); }); test('your own message is never folded', () => { - const rows = buildRows([user('first'), tool('a'), say('answer'), user('second')], false); + const rows = buildRows([user('first'), tool('a'), say('answer'), user('second')], false, 4); expect(rows[0]?.kind === 'message' && rows[0].message).toEqual(user('first')); }); - test('a window with no user message at all folds nothing', () => { - expect(shape([say('note'), tool('a'), say('answer')], false)).toEqual(['assistant', 'tool', 'assistant']); + test('a window with no user message still folds when it came from the server', () => { + // It used to stay open because there was no turn boundary to fold against. Under the history rule + // there is nothing to work out: it was loaded, so it is history. + expect(shape([say('note'), tool('a'), say('answer')], false)).toEqual(['fold', 'assistant']); + }); + + test('nothing folds while you are watching, however many turns you send', () => { + // The behaviour this whole rule exists for. Three complete turns, none of them loaded from the + // server: under the old rule the first two collapsed the instant the next message was sent. + const messages = [ + user('1'), + tool('a'), + say('one'), + user('2'), + tool('b'), + say('two'), + user('3'), + tool('c'), + say('three'), + ]; + expect(shape(messages, false, 0)).toEqual([ + 'user', + 'tool', + 'assistant', + 'user', + 'tool', + 'assistant', + 'user', + 'tool', + 'assistant', + ]); + }); + + test('the boundary is where the server stopped and this sitting began', () => { + // First turn loaded (6 messages), second typed here. The loaded one folds; the new one does not. + const messages = [user('1'), tool('a'), say('one'), user('2'), tool('b'), say('two')]; + expect(shape(messages, false, 3)).toEqual(['user', 'fold', 'assistant', 'user', 'tool', 'assistant']); + }); + + test('a turn still generating is never folded, even reloaded onto mid-turn', () => { + // Reload while a turn is in flight: every message is history, but hiding the work as it arrives is + // the exact thing being undone. The finished turn above it folds as usual. + const messages = [user('1'), tool('a'), say('one'), user('2'), tool('b')]; + expect(shape(messages, true, messages.length)).toEqual(['user', 'fold', 'assistant', 'user', 'tool']); }); test('every older turn folds, not just the previous one', () => { @@ -117,6 +170,7 @@ describe('buildRows', () => { tool('c'), say('three'), ]; + // Including the last one, which used to be exempt for being the live turn. Reloaded, nothing is live. expect(shape(messages, false)).toEqual([ 'user', 'fold', @@ -125,7 +179,7 @@ describe('buildRows', () => { 'fold', 'assistant', 'user', - 'tool', + 'fold', 'assistant', ]); }); @@ -141,24 +195,27 @@ describe('buildRows', () => { /** What tool rows key their expansion off. Getting this wrong expands a whole transcript at once. */ describe('buildRows — the live flag', () => { - const live = (messages: ChatMessage[], isGenerating = false) => - buildRows(messages, isGenerating) + const live = (messages: ChatMessage[], isGenerating = false, historicalCount = messages.length) => + buildRows(messages, isGenerating, historicalCount) .filter((r) => r.kind === 'message') .map((r) => (r.kind === 'message' ? r.live : null)); - test('the current turn is live, and stays live after it finishes', () => { - expect(live([user('go'), tool('a'), say('answer')], true)).toEqual([true, true, true]); - expect(live([user('go'), tool('a'), say('answer')], false)).toEqual([true, true, true]); + test('a turn from this sitting is live, generating or finished', () => { + // Finishing is not what ends a turn's expansion any more — being reloaded is. + expect(live([user('go'), tool('a'), say('answer')], true, 0)).toEqual([true, true, true]); + expect(live([user('go'), tool('a'), say('answer')], false, 0)).toEqual([true, true, true]); }); - test('a superseded turn is not live — only your question and my answer survive as rows', () => { - expect(live([user('first'), tool('a'), say('answer'), user('second')])).toEqual([false, false, true]); + test('a folded turn is not live — only your question and my answer survive as rows', () => { + expect(live([user('first'), tool('a'), say('answer'), user('second')])).toEqual([false, false, false]); }); - test('a window with no user message is unfolded but not live', () => { - // Nothing is current when there is no boundary to find which turn is. Marking it live would open every - // tool call in a lazily-loaded chunk the moment it scrolled into view. - expect(live([say('note'), tool('a'), say('answer')])).toEqual([false, false, false]); + test('unfolded and live are now one claim', () => { + // They came apart only because the old rule force-unfolded a window it could find no turn boundary in, + // and opening every tool call in a lazily-loaded chunk would have been wrong. That window folds now, + // so there is no case left where a row is on screen unfolded but not current. + expect(live([say('note'), tool('a'), say('answer')])).toEqual([false]); + expect(live([say('note'), tool('a'), say('answer')], false, 0)).toEqual([true, true, true]); }); }); diff --git a/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.ts b/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.ts index cbb0382a..efff6af0 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.ts +++ b/src/workspaces/officerdev/src/apps/Chat/components/turn-rows.ts @@ -1,15 +1,19 @@ import type { ChatMessage } from '../types'; /** - * A conversation is a chess game: your message, my work, my answer, repeat. Only the turn you are in is - * worth seeing in full — the moment you send the next message, the tool calls and the running commentary - * that produced the previous answer stop being the thing you are reading and start being the thing you are - * scrolling past. So every turn but the live one folds to three parts: what you asked, one summary row, and - * what I concluded. + * A finished conversation is worth reading as question, summary, answer — the tool calls and running + * commentary that produced an answer are the thing you scroll past. So a turn folds to those three parts. * - * This is a pure derivation over the message list, which is what makes a reload render identically to a - * live session: there is no fold *state* anywhere, on the wire or on disk, only a grouping that both paths - * compute from the same array. + * But it folds because it is HISTORY, not because a newer turn exists. That distinction is the whole + * design: this used to collapse the previous turn the instant you sent the next message, and watching + * your own conversation fold up under you as you typed was worse than the scrolling it saved. Everything + * that happened while you were watching stays open for as long as the tab is; reload, and all of it has + * become history and folds at once. + * + * The cost is a property worth naming, because it was deliberate before and is deliberately given up now: + * folding is no longer a pure derivation over the message list, so a reload does NOT render identically to + * a live session. That is the feature. The state it depends on is a single count of how many messages came + * from the server — it lives in `useChat`, never on the wire and never on disk. */ /** Rows that are landmarks about the conversation rather than work done inside a turn. These never fold. */ @@ -41,9 +45,8 @@ export type FoldCounts = { export type ChatRow = /** - * `live` marks the turn that is happening now — the one after your most recent message. It is what tool - * rows key their default expansion off, so "expanded" is a fact about where the row sits in the - * conversation rather than about how long ago it rendered. + * `live` marks a turn from this sitting rather than one loaded from the server. It is what tool rows key + * their default expansion off, so "expanded" means "you were here when this happened". */ | { kind: 'message'; key: string; message: ChatMessage; live: boolean } /** A collapsed run of one turn's work. `messages` is the whole of it, in order, for when it is opened. */ @@ -78,11 +81,11 @@ type TurnParams = { segment: ChatMessage[]; /** Index of `segment[0]` in the full list, so keys stay unique across turns. */ offset: number; - /** False for the live turn: its work stays on screen. */ + /** False for a turn from this sitting: its work stays on screen. */ fold: boolean; /** Whether the turn has finished — the live turn has not, while a response is still streaming. */ complete: boolean; - /** Stamped onto every row of this turn. Not simply `!fold` — see `buildRows`. */ + /** Stamped onto every row of this turn. Now simply `!fold` — see `buildRows`. */ live: boolean; }; @@ -146,9 +149,14 @@ function buildTurn({ segment, offset, fold, complete, live }: TurnParams): ChatR } /** - * Group a flat transcript into display rows. Only the turn after your last message stays open. + * `historicalCount` is how many messages at the front of the list came from the server rather than from + * this sitting. It is what decides folding now: a turn folds because it is history, not because a newer + * turn exists. Everything that happened while you were watching stays open, however many turns you send + * after it; reload, and all of it is history and folds together. + * + * Omitting it folds nothing, which is the right default for a caller that cannot tell the two apart. */ -export function buildRows(messages: ChatMessage[], isGenerating: boolean): ChatRow[] { +export function buildRows(messages: ChatMessage[], isGenerating: boolean, historicalCount = 0): ChatRow[] { if (!messages.length) return []; const userAt: number[] = []; @@ -156,34 +164,33 @@ export function buildRows(messages: ChatMessage[], isGenerating: boolean): ChatR if (m.role === 'user') userAt.push(i); }); - // Everything from your most recent message onward is live. A transcript that opens mid-conversation may - // have no user message at all in its window; then nothing has been superseded and nothing folds. - const liveStart = userAt.length ? userAt[userAt.length - 1]! : 0; - - const bounds = [0, ...userAt.filter((i) => i > 0 && i < liveStart), liveStart].filter( - (v, i, a) => i === 0 || v !== a[i - 1], - ); + // Every user message is a turn boundary — including the last one, which used to be excluded because the + // turn after it was by definition the live one. Now the final segment is an ordinary turn that folds or + // not on the same rule as the rest. + const bounds = [0, ...userAt.filter((i) => i > 0), messages.length].filter((v, i, a) => i === 0 || v !== a[i - 1]); const rows: ChatRow[] = []; for (let i = 0; i < bounds.length - 1; i++) { const from = bounds[i]!; const to = bounds[i + 1]!; + const isLast = to === messages.length; + // Fold only a turn whose every message is history. A turn still generating is never folded even when + // its opening messages came from the server — that is a reload onto a live turn, and hiding the work + // as it arrives is exactly the thing being undone here. + const generatingNow = isLast && isGenerating; + const fold = to <= historicalCount && !generatingNow; rows.push( - ...buildTurn({ segment: messages.slice(from, to), offset: from, fold: true, complete: true, live: false }), + ...buildTurn({ + segment: messages.slice(from, to), + offset: from, + fold, + complete: !generatingNow, + // Unfolded and live are one claim now: if a turn's work is on screen, its tool rows are open. The + // two came apart only because the old rule force-unfolded a window it could find no boundary in. + live: !fold, + }), ); } - rows.push( - ...buildTurn({ - segment: messages.slice(liveStart), - offset: liveStart, - fold: false, - complete: !isGenerating, - // Unfolded is not the same claim as live. With no user message in the window there is no turn boundary - // to find, so the whole window stays unfolded — and calling all of it live would expand every tool call - // in it at once. Nothing is current when we cannot tell which turn is. - live: userAt.length > 0, - }), - ); return rows; } diff --git a/src/workspaces/officerdev/src/hooks/useChat.ts b/src/workspaces/officerdev/src/hooks/useChat.ts index 40592ac7..57c98b2b 100644 --- a/src/workspaces/officerdev/src/hooks/useChat.ts +++ b/src/workspaces/officerdev/src/hooks/useChat.ts @@ -61,6 +61,19 @@ export function useChat(initialSessionId?: string, initialModel?: string | null, onTurnComplete, } = options ?? {}; const [messages, setMessages] = useState(preloadedMessages ?? []); + /** + * How many messages at the FRONT of the list came from the server rather than from this sitting. + * + * Folding used to be a pure derivation over the array, which is what made a reload render identically to + * a live session. That property is deliberately given up here: a turn now folds because it is *history*, + * not because a newer turn exists. Watching your own conversation collapse under you as you typed the + * next message was the thing worth losing it for. + * + * A count rather than a set of ids because everything historical is contiguous and at the front: the + * preload seeds it, paging older messages prepends to it, and everything live is appended past it. The + * one case that replaces rather than appends is a resume, where the whole transcript is history. + */ + const [historicalCount, setHistoricalCount] = useState(preloadedMessages?.length ?? 0); // Reverse pagination: absolute index of the oldest loaded message (0 = we have the whole transcript). const [oldestOffset, setOldestOffset] = useState(paginate?.initialOffset ?? 0); const [isLoadingOlder, setIsLoadingOlder] = useState(false); @@ -135,6 +148,9 @@ export function useChat(initialSessionId?: string, initialModel?: string | null, ); if (res.messages.length) { setMessages((prev) => [...res.messages, ...prev]); + // Paged-in messages are older than everything on screen, so they join the historical block at the + // front and the boundary between history and this sitting moves with them. + setHistoricalCount((count) => count + res.messages.length); setOldestOffset(res.offset); } else { setOldestOffset(0); @@ -366,6 +382,9 @@ export function useChat(initialSessionId?: string, initialModel?: string | null, setSessionId(msg.sessionId); const transcript = rebuildTranscript(msg.messages); setMessages(transcript); + // A resume replaces the list wholesale with what the server has. All of it is history — including a + // turn this tab watched arrive before it disconnected, which is now something being read back. + setHistoricalCount(transcript.length); if (transcript.length > 0) setHasStarted(true); setIsGenerating(msg.isGenerating); if (msg.streamingText) { @@ -595,6 +614,7 @@ export function useChat(initialSessionId?: string, initialModel?: string | null, return { messages, + historicalCount, streamingText, isConnected, isGenerating,