diff --git a/CHANGELOG.md b/CHANGELOG.md index 26217bd..9c1cbf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,20 @@ reach the delegation that made them. ### Changed +- **The conversation only shows what was actually said.** When the agent spent a + step running commands without writing anything, the transcript still drew a + row for it: an icon, a clock and a small work pill with no sentence beside + them. A long task could put half a dozen of those between one reply and the + next, and skimming the chat meant stepping over rows that said nothing. + + Those steps no longer appear in the conversation. The moment a written reply + arrives, it carries the pill for everything that led up to it — "3 commands · + 1 reasoning · 8.1s" on the sentence that came out of them — and opening it + lands on the trace at the *start* of that stretch rather than at the reply's + own last command. Nothing is hidden: the trace holds every call exactly as + before, and a trace row or a search hit that points at a suppressed step now + scrolls to the reply that speaks for it. + - **A phone gets a layout built around the conversation.** It used to be the desktop layout at the same size: the figures you read mid-session — the cost, the model, the state, whether approvals are bypassed — were set smaller than @@ -78,6 +92,11 @@ past the space it had and was painted over the two things below it. ### Internal +- A browser check covers the suppressed steps end to end — that they leave no + row, that the trace still holds them, and that clicking the reply's pill lands + on the first of them. The suite's virtual-time budget grew with it: a run that + outgrows the budget reports no results rather than a failure, so it now has + room over what the checks need. - The automated browser checks run at phone viewports (portrait, keyboard-open and landscape), with each of the phone's own disclosures and its menu open in turn, and assert the geometry rather than the intent: target size, the space diff --git a/src/client/shell/chat/ChatView.tsx b/src/client/shell/chat/ChatView.tsx index f9b0e8f..4cb372e 100644 --- a/src/client/shell/chat/ChatView.tsx +++ b/src/client/shell/chat/ChatView.tsx @@ -4,6 +4,7 @@ import { uploadAttachment } from '../../chat/attachments-api.js'; import { ChatController, type ChatUnavailable } from '../../chat/controller.js'; import { fetchStatus, findFiles } from '../../chat/workspace-api.js'; import { activityEvents } from '../../chat/activity.js'; +import type { ChatTranscript } from '../../chat/transcript.js'; import { groupTurns, isTurnOpen, turnOf, type TurnSummary } from '../../chat/turns.js'; import type { ChatPanelId, ChatViewSettings } from '../../chat/view-settings.js'; import { @@ -20,7 +21,7 @@ import { PhoneContext } from '../../ui/touch.js'; import { FloatingMenu } from '../FloatingMenu.js'; import { Composer } from './Composer.js'; import { MessageList, type MessageListHandle } from './MessageList.js'; -import { messageText } from './MessageBubble.js'; +import { hasVisibleContent, messageText } from './MessageBubble.js'; import { PermissionCard } from './PermissionCard.js'; import { PlanPanel } from './PlanPanel.js'; import { SessionHeader } from './SessionHeader.js'; @@ -367,13 +368,18 @@ export function ChatView({ (messageId: string) => { const turn = turnOf(messageId, turns); if (turn) openTurn(turn.id); + // A step that only ran commands has no row of its own (issue #46), and a + // trace row or a search hit pointing at one would scroll to an element + // that is not in the document — a click that does nothing at all. The + // reply that speaks for it is the row to land on instead. + const target = rowFor(messageId, turn, transcript); // A turn that was just opened has not painted yet — its bubbles have no // layout box to scroll to until the next tick. A turn already open // scrolls one tick later than it strictly needs to, which nothing on // screen can tell apart from immediate. - window.setTimeout(() => list.current?.scrollToMessage(messageId), 0); + window.setTimeout(() => list.current?.scrollToMessage(target), 0); }, - [turns, openTurn], + [turns, openTurn, transcript], ); const copyTurn = React.useCallback( @@ -904,6 +910,40 @@ export function ChatView({ const ZERO = (): number => 0; +/** + * The row that stands for a message on screen. + * + * Itself, unless it is a step that said nothing — then the next message in its + * turn that did, because that is the one carrying its work pill. Failing that, + * the last one before it, so a turn whose tail is all machinery still lands + * somewhere. Falls back to the id given, which scrolls nowhere and is exactly + * what happened before this existed. + * + * Exported so the rule can be asserted directly: everything it guards happens + * inside a `setTimeout` around a scroll, which a rendered tree cannot show. + */ +export function rowFor( + messageId: string, + turn: TurnSummary | undefined, + transcript: ChatTranscript, +): string { + const message = transcript.message(messageId); + if (!turn || !message || hasVisibleContent(message)) return messageId; + + const ids = turn.messageIds; + const at = ids.indexOf(messageId); + if (at < 0) return messageId; + + const visible = (id: string): boolean => { + const other = transcript.message(id); + return Boolean(other && hasVisibleContent(other)); + }; + const after = ids.slice(at + 1).find(visible); + if (after) return after; + const before = ids.slice(0, at).filter(visible).pop(); + return before || messageId; +} + function viewportHeight(): number { return typeof window === 'undefined' ? 900 : window.innerHeight; } diff --git a/src/client/shell/chat/MessageBubble.tsx b/src/client/shell/chat/MessageBubble.tsx index 6cc3ea7..1bffe6a 100644 --- a/src/client/shell/chat/MessageBubble.tsx +++ b/src/client/shell/chat/MessageBubble.tsx @@ -32,6 +32,13 @@ import { PlanPanel } from './PlanPanel.js'; * they can be read as a sequence of work rather than as interruptions in the * middle of a sentence. What is left behind is a work pill: how much happened, * and one click to go and look at it. Moved, never hidden. + * + * A step that was *only* machinery therefore has nothing left to say, and gets + * no row at all — a glyph, a clock and a pill with no sentence beside them is a + * row the eye has to stop on to learn that nothing was said. Its work is not + * lost: the list hands those ids to the next message that does speak (see + * `carriedIds`), whose pill counts them and whose "show work" lands on the + * first of them. The rail keeps every event either way. */ export interface MessageBubbleProps { @@ -48,6 +55,14 @@ export interface MessageBubbleProps { onRetry?: (messageId: string) => void; /** Open the rail and scroll its timeline to this message's first event. */ onShowWork?: (messageId: string) => void; + /** + * Ids of the silent steps this message speaks for, oldest first, comma-joined. + * + * A string rather than an array because it is a prop of a `React.memo` + * component: a fresh array per render would compare unequal every time and + * re-render the whole transcript on every streamed token. + */ + carriedIds?: string; /** Put this turn's text back in the composer, unsent. */ onEdit?: (text: string) => void; /** Count reasoning blocks in the work pill, per the chat display settings. */ @@ -63,16 +78,32 @@ export const MessageBubble = React.memo(function MessageBubble({ onRetry, onShowWork, onEdit, + carriedIds = '', showThinking = true, showToolCalls = true, }: MessageBubbleProps) { const id = message.id; + // This message, plus the silent steps it speaks for — a tool call can report + // how long it took after the message it belongs to has closed and the next + // one has opened, and a pill that inherited that call has to hear about it. + // Still one bubble per event rather than the whole list: nothing here widens + // to the transcript. + const watched = React.useMemo(() => [id, ...splitIds(carriedIds)], [id, carriedIds]); + const subscribe = React.useCallback( - (listener: () => void) => transcript.subscribeMessage(id, listener), - [transcript, id], + (listener: () => void) => { + const offs = watched.map((each) => transcript.subscribeMessage(each, listener)); + return () => { + for (const off of offs) off(); + }; + }, + [transcript, watched], + ); + const getVersion = React.useCallback( + () => watched.reduce((sum, each) => sum + transcript.getMessageVersion(each), 0), + [transcript, watched], ); - const getVersion = React.useCallback(() => transcript.getMessageVersion(id), [transcript, id]); // Static rendering has no subscription to read from, so the server snapshot // is a constant; the third argument is required or React throws there. const version = React.useSyncExternalStore(subscribe, getVersion, ZERO); @@ -95,13 +126,23 @@ export const MessageBubble = React.memo(function MessageBubble({ && current.blocks.length > 0 && current.blocks.every((block) => block.kind === 'notice'); - // Derived from this message's own blocks, not from a list handed down. An - // events array as a prop would be a new object identity every render and - // would defeat React.memo for the whole transcript on every streamed token. + // Derived from blocks, not from a list handed down. An events array as a prop + // would be a new object identity every render and would defeat React.memo for + // the whole transcript on every streamed token — which is also why the silent + // steps this message speaks for arrive as a joined string of ids. + // + // Those steps are resolved through the transcript rather than subscribed to: + // a step only becomes silent-and-carried once the message after it has opened, + // by which time nothing is still streaming into it. const work = React.useMemo( - () => summariseWork(current, showThinking, showToolCalls), + () => { + const carried = splitIds(carriedIds) + .map((carriedId) => transcript.message(carriedId)) + .filter((message): message is ChatMessage => Boolean(message)); + return summariseWork([...carried, current], showThinking, showToolCalls); + }, // eslint-disable-next-line react-hooks/exhaustive-deps - [current, version, showThinking, showToolCalls], + [transcript, current, version, carriedIds, showThinking, showToolCalls], ); const copy = React.useCallback(() => { @@ -119,12 +160,22 @@ export const MessageBubble = React.memo(function MessageBubble({ }); }, [current]); - // Nothing left to draw: an assistant turn that was only machinery, with both - // display toggles off. An empty bordered row is worse than no row — it reads - // as a message that failed to render rather than as one the settings hid. - if (!isUser && !isMarker && !current.streaming && visibleBlocks(current) === 0 && work.total === 0) { - return null; - } + // Nothing to say: a step that was only machinery. Its tool calls and its + // reasoning are on the rail, and its count is carried onto the next message + // that does speak, so the row itself would be an empty bordered strip with a + // clock on it — indistinguishable from a message that failed to render. + // + // Independent of the display settings, and independent of `streaming`: a step + // that has so far produced only tool calls is exactly the case this is about, + // and the live ribbon is what says the agent is working while it does. + // + // The one row kept: a message that has opened and produced *nothing* yet, + // while streaming. That is the caret between sending and the first block + // arriving, and it is a reply about to happen rather than machinery. + // + // The rule itself lives in `hasVisibleContent` because the list decides which + // ids to carry with it, and the two answers must be the same one. + if (!hasVisibleContent(current)) return null; if (isMarker) { return ( @@ -192,7 +243,10 @@ export const MessageBubble = React.memo(function MessageBubble({ {current.streaming && visibleBlocks(current) === 0 ? : null} {!isUser && work.total > 0 && onShowWork ? ( - onShowWork(id)} /> + // Aimed at the earliest step it counts, not at this message: the point + // of the pill on a reply that follows silent work is to open the trace + // at the *start* of that stretch. + onShowWork(work.firstId || id)} /> ) : null} {isUser ? null :