diff --git a/app/src/components/channels/channel-chat.tsx b/app/src/components/channels/channel-chat.tsx index 8f4546a22..d16b83506 100644 --- a/app/src/components/channels/channel-chat.tsx +++ b/app/src/components/channels/channel-chat.tsx @@ -141,6 +141,33 @@ export function ChannelChat({ const [runError, setRunError] = useState(null); const awaitingReply = useRef(false); + /* + * TWO DIFFERENT FACTS ABOUT ONE TURN, AND NEITHER OF THEM IS `agent.isRunning`. + * + * `turnsInFlight` counts what a person would call the Bot having the turn: from the moment `say` + * is entered until the whole thing has come back, browser actions in the middle included. It is + * what decides whether the next thing typed is sent or parked, and what tells the queue its wait + * is over. + * + * `runsInFlight` counts what Stop can actually reach: the run `copilotkit.runAgent` opens, and + * nothing before it. A turn can be in flight for a second and a half before that, while `say` + * waits for the runtime agent, and a Stop drawn in that window aborts a controller nobody has + * made yet. + * + * `agent.isRunning` looks like both and is neither. It reports the run on the wire, and a turn + * that touches the browser is several runs in a row: the Bot asks for a click, the run ENDS so + * the browser can answer it, and another run starts carrying the answer. The agent reports itself + * idle in every one of those gaps — the truth about the wire and a lie about the turn. OpenBot + * registers every computer tool as a frontend tool, so the gaps open on ordinary work rather than + * on some edge case, and anything keyed on the turn ending fires in the middle of one instead. + * + * Counters rather than booleans because nothing stops a second turn being started from a + * component button while the first is still going, and two overlapping turns must not have the + * first one to finish declare the conversation idle. + */ + const [turnsInFlight, setTurnsInFlight] = useState(0); + const [runsInFlight, setRunsInFlight] = useState(0); + /** * Tell the roster what was just said. Failures here must not block the conversation. */ @@ -159,12 +186,10 @@ export function ChannelChat({ reportRef.current = report; /** - * Send a user turn through the channel, including activity reporting and history repair. + * Everything `say` does once it has something worth sending, split out so the counter it is + * wrapped in covers every way out of here, a throw included. */ - const say = async (text: string, skillInstructions: string[] = []) => { - const trimmed = text.trim(); - if (!trimmed) return; - + const deliver = async (trimmed: string, skillInstructions: string[]) => { // Wait briefly for the runtime agent instance before adding the message. if (!isReadyRef.current) { await Promise.race([ @@ -211,7 +236,32 @@ export function ChannelChat({ agent.setMessages(repaired as typeof agent.messages); } - await copilotkit.runAgent({ agent }); + setRunsInFlight((count) => count + 1); + try { + await copilotkit.runAgent({ agent }); + } finally { + setRunsInFlight((count) => count - 1); + } + }; + + /** + * Send a user turn through the channel, including activity reporting and history repair. + * + * Every user turn in this channel goes through here — what the composer sends, the seed from the + * compose screen, and a button inside a rendered component. That is what makes the counter worth + * keeping here rather than in the view: the view sees only the turns it started itself, and a + * queue that drains on the wrong one of those posts a correction into the middle of an answer. + */ + const say = async (text: string, skillInstructions: string[] = []) => { + const trimmed = text.trim(); + if (!trimmed) return; + + setTurnsInFlight((count) => count + 1); + try { + await deliver(trimmed, skillInstructions); + } finally { + setTurnsInFlight((count) => count - 1); + } }; useEffect(() => { @@ -335,7 +385,26 @@ export function ChannelChat({ awaitingReply.current = false; copilotkit.stopAgent({ agent }); }} - pending={agent.isRunning} + /* + * The turn, not the run. A browser action ends one run and starts another, and telling the + * conversation it is idle in between is what would drain a parked correction into the + * middle of an answer: a second turn racing the first on one thread, with a fabricated + * result stitched over a tool call that is still executing. + */ + pending={agent.isRunning || turnsInFlight > 0} + /* + * A channel outlives its turns, so it is the screen where waiting is worth offering. A + * correction typed mid-answer is held here, in this tab, and runs as one follow-up turn the + * moment this one is over — including when it is over because somebody pressed the button + * above. + */ + queueWhileBusy + /* + * The run, not the turn. Stop reaches a run through the core's abort controller, and that + * controller does not exist until `say` has finished waiting for the runtime agent — so + * this is the one place the narrower fact is the honest one to draw a button from. + */ + stoppable={agent.isRunning || runsInFlight > 0} /> ); diff --git a/app/src/components/channels/chat-transcript.tsx b/app/src/components/channels/chat-transcript.tsx index 20d4c0c7a..89406dc11 100644 --- a/app/src/components/channels/chat-transcript.tsx +++ b/app/src/components/channels/chat-transcript.tsx @@ -7,7 +7,11 @@ import { Streamdown } from "streamdown"; import { markdownComponents } from "@/lib/markdown"; import { EASE_OUT, ENTRANCE_SECONDS } from "@/lib/motion"; import { Bubble, BubbleContent } from "@/components/ui/bubble"; -import { MessageContent, Message as MessageRow } from "@/components/ui/message"; +import { + MessageContent, + MessageFooter, + Message as MessageRow, +} from "@/components/ui/message"; import { MessageScroller, MessageScrollerButton, @@ -15,8 +19,10 @@ import { MessageScrollerItem, MessageScrollerProvider, MessageScrollerViewport, + useMessageScroller, } from "@/components/ui/message-scroller"; import { toVisibleChatItems } from "./chat-messages"; +import type { QueuedMessage } from "./composer"; import { ToolLine } from "./tool-line"; import { ToolRenderBoundary } from "./tool-boundary"; @@ -25,8 +31,18 @@ type ChatTranscriptProps = { /** Comma-separated `/` command names, used to tell a skill chip from a leading slash. */ commandNames?: string; messages: ReadonlyArray>; + /** + * Typed while the Bot had the turn, and waiting for it to finish. Empty on a screen that does not + * offer queueing at all. + */ + queued?: readonly QueuedMessage[]; + /** Take one back before it runs. Without it a queued line is shown but cannot be undone. */ + onRemoveQueued?: (id: string) => void; }; +/** One shared empty array, so a screen without a queue does not hand down a new one per render. */ +const EMPTY_QUEUE: readonly QueuedMessage[] = []; + /** * Split a person's message into the skill they invoked and the rest of what they typed. * @@ -75,6 +91,102 @@ function Thinking() { ); } +/** + * Something the person said while the Bot was working, waiting its turn. + * + * IT IS DRAWN AS THEIR MESSAGE, NOT AS A NOTICE ABOUT ONE. The whole point of letting somebody type + * mid-turn is that they can see their words landed, and a status line saying "1 message queued" + * does not do that — they would still be wondering whether the sentence they typed is the sentence + * that will run. So it is the same bubble, in the same column, with the same wrapping, and only two + * things say it has not run yet: it is faded, and it says so underneath. + * + * The footer carries the taking-back too, because that is where the reader's eye already is once + * they have decided this was a mistake, and because a control on the bubble itself would have to + * hover over the words it is offering to delete. + */ +function Queued({ + text, + onRemove, +}: { + text: string; + onRemove?: (() => void) | undefined; +}) { + return ( + + + + + {/* Shown exactly as typed, for the same reason a sent message is. */} + {text} + + + + {/* + * `status` rather than `alert`, matching the thinking line: a person who has just chosen + * to queue something is not being interrupted by the news that it is queued. + */} + Queued + {onRemove ? ( + + ) : null} + + + + ); +} + +/** + * Put the newest queued message where the person who just typed it can see it. + * + * WITHOUT THIS THE AFFORDANCE IS INVISIBLE EXACTLY WHEN IT MATTERS. The scroller holds its anchor on + * the turn being answered rather than following the bottom, so during a long streamed answer the + * transcript sits a screen or so above the end — and a line appended below it lands off screen. + * Measured at the point somebody would actually use this: eighty-odd pixels under the fold, with + * the composer emptying at the same moment. They would have watched their correction vanish. + * + * Keyed on the newest queued id rather than on the list, so it does not fire again for every chunk + * of the answer still streaming above it. It does fire when the bottom-most queued line is taken + * back, which is a scroll nobody asked for and which lands on the end of the conversation anyway, + * and it stays quiet on a drain, when the id goes to null. + * + * IT COSTS THE ANCHOR, AND THAT IS THE PRICE OF THE SCROLL RATHER THAN A SIDE EFFECT OF IT. + * `scrollToEnd` drops whatever turn the scroller was holding its position against and starts + * following the bottom instead, so the rest of that answer streams past under the reader rather + * than staying put beneath the question. Somebody who has just typed at the bottom of the + * conversation has asked to be at the bottom of the conversation, so following it is the reading + * they chose; but they chose it for the whole turn and not only for the moment, and the button + * back to the anchored view is the scroller's own, not ours to restore. + * + * Rendering nothing and living inside the provider is what buys access to the scroller at all; the + * alternative is threading a ref out through three components with no other reason to know a + * scroller exists. + */ +function ScrollNewestQueuedIntoView({ newest }: { newest: string | null }) { + const { scrollToEnd } = useMessageScroller(); + + useEffect(() => { + if (newest === null) { + return; + } + scrollToEnd(); + }, [newest, scrollToEnd]); + + return null; +} + /** * How many of the newest turns cascade when a channel is opened, and how far apart. * @@ -341,6 +453,8 @@ export function ChatTranscript({ busy = false, commandNames = "", messages, + onRemoveQueued, + queued = EMPTY_QUEUE, }: ChatTranscriptProps) { /* * NOT MEMOISED, AND THAT IS DELIBERATE. `useMemo` keyed on `messages` looks obviously right and @@ -436,9 +550,26 @@ export function ChatTranscript({ * the scroller to measure and anchor something that exists for a second and a half. */} {waitingOnFirstToken ? : null} + {/* + * Below the thinking line, and outside the item list for the same reason it is: these + * are not yet turns. They have ids of their own, but they are this tab's ids and not the + * thread's, so handing them to the scroller would ask it to anchor on something that is + * about to be replaced by a message with a different id — and the replacement is the + * one worth scrolling to. + */} + {queued.map((message) => ( + onRemoveQueued(message.id) : undefined + } + text={message.text} + /> + ))} + ); diff --git a/app/src/components/channels/composer/composer.tsx b/app/src/components/channels/composer/composer.tsx index 7ce029b1c..0c4781b28 100644 --- a/app/src/components/channels/composer/composer.tsx +++ b/app/src/components/channels/composer/composer.tsx @@ -43,6 +43,20 @@ export type ComposerProps = { * structured data instead of something it would have to re-parse out of the text. */ onSubmit?: (draft: ComposerDraft) => void | Promise; + /** + * Park this message until the turn in flight is over, instead of refusing the keystroke. + * + * Its presence is what lets a person type at a Bot that is already working. Without it the + * composer goes on refusing mid-turn sends, which is still the right answer for a screen that has + * nowhere to put a parked message — the compose screen creates the channel on send and then + * navigates away, so anything parked there would be dropped on unmount, and a message that + * silently disappears is worse than a send button that visibly will not go. + * + * Called instead of `onSubmit`, not as well as it, and it does not return a promise: parking is + * a state change, and awaiting one would hold the composer's send lock for the length of somebody + * else's turn and block the next correction. + */ + onQueue?: (draft: ComposerDraft) => void; /** Stop the Bot mid-answer; while pending, the send button becomes a stop button. */ onStop?: () => void; /** @@ -52,10 +66,23 @@ export type ComposerProps = { */ disabled?: boolean; /** - * A run is in flight. It gates sending, not writing: a channel is `pending` while it is still + * A turn is in flight. It gates sending, not writing: a channel is `pending` while it is still * connecting and restoring its history, and the composer is on screen throughout. */ pending?: boolean; + /** + * There is a run on the wire for Stop to reach. + * + * Not the same question as `pending`, and telling them apart is the whole reason this exists. A + * turn is in flight from the moment somebody presses send; the run it becomes does not exist + * until the caller has waited for whatever it has to wait for, which on a channel that is still + * joining is up to a second and a half. A Stop button drawn in that window aborts a controller + * nobody has made yet: the press is swallowed, the message goes anyway, and the one control the + * whole affordance leans on has quietly lied. + * + * Defaults to `pending`, which is the right answer for a caller with no gap between the two. + */ + stoppable?: boolean; }; export function Composer({ @@ -64,9 +91,11 @@ export function Composer({ agents = [], commands = PLACEHOLDER_COMMANDS, onSubmit, + onQueue, onStop, disabled = false, pending = false, + stoppable, }: ComposerProps) { const [value, setValue] = useState([]); const [isSubmitting, setIsSubmitting] = useState(false); @@ -107,13 +136,32 @@ export function Composer({ const submitDraft = useCallback( async (segments: Segment[]) => { const submitted = toDraft(segments); - if ( - submitted.isEmpty || - disabled || - isBusy || - submitInFlight.current || - !onSubmit - ) { + if (submitted.isEmpty || disabled) { + return; + } + + /* + * A TURN IS IN FLIGHT, AND THIS IS THE FORK THE WHOLE AFFORDANCE HANGS ON. + * + * With somewhere to park it the message goes there and the box empties, so the person sees + * their words land. Without, we are back to refusing, which is what every caller that does + * not queue still gets. + * + * It returns before `submitInFlight` and `isSubmitting` are touched on purpose. Those guard + * one send from starting twice; a send here is held open for the length of the whole run, so + * borrowing them for a parked message would let the first turn lock out every correction + * typed while it worked — the exact thing this exists to allow. + */ + if (isBusy) { + if (!onQueue) { + return; + } + setValue([]); + onQueue(submitted); + return; + } + + if (submitInFlight.current || !onSubmit) { return; } @@ -134,7 +182,7 @@ export function Composer({ wantsFocus.current = true; } }, - [disabled, isBusy, onSubmit], + [disabled, isBusy, onQueue, onSubmit], ); /** @@ -157,9 +205,36 @@ export function Composer({ void submitDraft(value); }; - const canSend = !disabled && !isBusy && !draft.isEmpty; - /** Stop is available only once the agent run is actually pending. */ - const canStop = Boolean(onStop) && pending; + /** + * There is a turn in flight and somewhere to park what is being typed. + * + * Not the same question as "is anything typed" — an empty composer mid-turn can queue nothing, + * and the button it wants is Stop. + */ + const canQueue = Boolean(onQueue) && isBusy && !disabled; + /** Something is typed, mid-turn, with a queue to put it in. */ + const parking = canQueue && !draft.isEmpty; + const canSend = !disabled && !draft.isEmpty && (!isBusy || canQueue); + /** + * Stop is available only once there is a run for it to reach, and it gives way to Send the moment + * there is something typed to park. + * + * `stoppable` rather than `pending`, because a turn is in flight before its run is, and a button + * that cannot do the thing it names is worse than no button at all. + * + * One button, so one of the two has to yield. Send wins because the correction is the thing that + * cannot wait: park it and the box empties, which brings Stop straight back — so stopping is + * never more than one press away, and the press before it is the one that saves the sentence. + * Showing both would be honest and would also put two round buttons in a row on a compact + * composer that has room for one. + */ + const canStop = Boolean(onStop) && (stoppable ?? pending) && !parking; + /** + * The same arrow either way, because it is the same gesture, but a screen reader is told which of + * the two it is about to do. "Send" on a button that will not send for another minute is a small + * lie told to exactly the people who cannot see the queue it lands in. + */ + const sendLabel = parking ? "Queue message" : "Send message"; if (compact) { return ( @@ -217,7 +292,7 @@ export function Composer({ ) : (