speaking mid-turn no longer declares the turn over
Reported as "the output I see with turn complete is very very misleading", and it
was: the transcript printed "Turn finished · no closing message" between a tool
call and an injected message, while the header two inches away still said Working
and a task chip still said 1 RUNNING. The UI stated the opposite of what was
happening.
`turn-rows` treats every user message as a turn boundary. That was right when the
only way to speak was to wait. Delivering a queued message INTO a running turn made
it wrong: the boundary split the live turn, the half before it therefore counted as
`complete`, and having tool calls and no answer it matched exactly the rule that
prints a silent ending. The seam meant for a rare confusing case fired on an
ordinary one, and said something false.
So a user message now carries `injected` when it was sent while a turn was already
running, and only a message that STARTS a turn is a boundary. Read from
`isGeneratingRef` at send time rather than from state — `sendPrompt` is called
from closures built a render earlier, and the question is "was a turn running when
this was sent", which is what the ref holds.
Five tests, including the contrast case: the same messages WITHOUT the flag still
produce the boundary and the ending, because a genuine second turn should end the
first. A turn that really did finish silently still says so — that seam was worth
having and is not what was broken.
Verified live on the way in: the injection reached this session mid-turn, attached
to a tool result, and changed what the turn did.
Still outstanding from the same screenshot, not touched here: foreground Bash calls
render as background tasks ("Running in the background…", and a chip in the tray).
That is a separate misreport.
tsgo clean, frontend builds, 878 pass / 7 fail unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,8 @@ import type { ChatMessage } from '../types';
|
||||
import { buildRows, foldLabel } from './turn-rows';
|
||||
|
||||
const user = (text: string): ChatMessage => ({ role: 'user', text });
|
||||
/** Sent into a turn that was already running — see `injected` in types.ts. */
|
||||
const injected = (text: string): ChatMessage => ({ role: 'user', text, injected: true });
|
||||
const say = (text: string): ChatMessage => ({ role: 'assistant', id: text, text });
|
||||
const tool = (id: string, isError?: boolean): ChatMessage => ({
|
||||
role: 'tool',
|
||||
@@ -23,6 +25,56 @@ const shape = (messages: ChatMessage[], isGenerating = false, historicalCount =
|
||||
buildRows(messages, isGenerating, historicalCount).map((r) => (r.kind === 'message' ? r.message.role : r.kind));
|
||||
|
||||
describe('buildRows', () => {
|
||||
// ── Messages sent INTO a running turn ──
|
||||
//
|
||||
// The bug these pin: every user message was a turn boundary, so speaking mid-turn split the live turn
|
||||
// and the first half — tool calls, no answer yet — satisfied "finished without saying anything". The
|
||||
// reader got "Turn finished · no closing message" stamped across work that was still running, while the
|
||||
// header beside it said Working.
|
||||
|
||||
test('an injected message does not end the turn it was sent into', () => {
|
||||
expect(shape([user('go'), tool('a'), injected('also do X'), tool('b')], true, 0)).toEqual([
|
||||
'user',
|
||||
'tool',
|
||||
'user',
|
||||
'tool',
|
||||
]);
|
||||
});
|
||||
|
||||
test('the same messages WITHOUT the flag are what used to produce the false ending', () => {
|
||||
// Kept as the contrast: this is a genuine second turn, and ending the first one is correct there.
|
||||
expect(shape([user('go'), tool('a'), user('new question'), tool('b')], true, 0)).toEqual([
|
||||
'user',
|
||||
'tool',
|
||||
'turn-end',
|
||||
'user',
|
||||
'tool',
|
||||
]);
|
||||
});
|
||||
|
||||
test('a turn that really did end silently still says so', () => {
|
||||
expect(shape([user('go'), tool('a')], false, 0)).toEqual(['user', 'tool', 'turn-end']);
|
||||
});
|
||||
|
||||
test('an injected message still counts as part of the turn when it folds', () => {
|
||||
const messages = [user('go'), tool('a'), injected('also do X'), tool('b'), say('done')];
|
||||
const rows = buildRows(messages, false, messages.length);
|
||||
// One turn, so one fold — not two.
|
||||
expect(rows.filter((r) => r.kind === 'fold')).toHaveLength(1);
|
||||
expect(rows.some((r) => r.kind === 'turn-end')).toBe(false);
|
||||
});
|
||||
|
||||
test('several injections in one turn stay in one turn', () => {
|
||||
expect(shape([user('go'), tool('a'), injected('x'), tool('b'), injected('y'), say('done')], false, 0)).toEqual([
|
||||
'user',
|
||||
'tool',
|
||||
'user',
|
||||
'tool',
|
||||
'user',
|
||||
'assistant',
|
||||
]);
|
||||
});
|
||||
|
||||
test('the live turn is untouched — every row stays where it was', () => {
|
||||
expect(shape([user('go'), say('one'), tool('a'), say('two')], true)).toEqual([
|
||||
'user',
|
||||
|
||||
@@ -159,9 +159,16 @@ function buildTurn({ segment, offset, fold, complete, live }: TurnParams): ChatR
|
||||
export function buildRows(messages: ChatMessage[], isGenerating: boolean, historicalCount = 0): ChatRow[] {
|
||||
if (!messages.length) return [];
|
||||
|
||||
// A user message starts a turn — UNLESS it was sent into one that was already running.
|
||||
//
|
||||
// Queued messages can be delivered mid-turn: they reach the agent at its next step boundary and it
|
||||
// carries on with them. Treating those as boundaries split the live turn in two, which made the first
|
||||
// half `complete` — tool calls, no answer — and printed "Turn finished · no closing message" across a
|
||||
// turn that was visibly still working, with the header still saying so. Reported as "the output is very
|
||||
// misleading", and it was: the UI said the opposite of what was happening.
|
||||
const userAt: number[] = [];
|
||||
messages.forEach((m, i) => {
|
||||
if (m.role === 'user') userAt.push(i);
|
||||
if (m.role === 'user' && !m.injected) userAt.push(i);
|
||||
});
|
||||
|
||||
// Every user message is a turn boundary — including the last one, which used to be excluded because the
|
||||
|
||||
@@ -19,7 +19,19 @@ export type ModelOption = {
|
||||
};
|
||||
|
||||
export type ChatMessage =
|
||||
| { role: 'user'; text: string; images?: { filename: string; dataUrl: string }[] }
|
||||
| {
|
||||
role: 'user';
|
||||
text: string;
|
||||
images?: { filename: string; dataUrl: string }[];
|
||||
/**
|
||||
* Sent INTO a turn that was already running, rather than starting one.
|
||||
*
|
||||
* The distinction is invisible in the text and load-bearing in the layout: `turn-rows` treats every
|
||||
* user message as a turn boundary, so without this an injected message ends the turn it was sent
|
||||
* into — printing "Turn finished · no closing message" under work that was still going.
|
||||
*/
|
||||
injected?: true;
|
||||
}
|
||||
| { role: 'assistant'; id?: string; text: string }
|
||||
| { role: 'system'; text: string }
|
||||
| {
|
||||
|
||||
@@ -576,9 +576,19 @@ export function useChat(initialSessionId?: string, initialModel?: string | null,
|
||||
setHasStarted(true);
|
||||
}
|
||||
|
||||
// Read BEFORE `setIsGenerating(true)` below, and from the ref rather than the state: this function is
|
||||
// called from a closure that may have been built a render ago, and the answer has to be "was a turn
|
||||
// running when this was sent", which is exactly what the ref holds.
|
||||
const intoRunningTurn = isGeneratingRef.current;
|
||||
|
||||
setMessages((prev) => [
|
||||
...prev,
|
||||
{ role: 'user', text: displayText ?? text, ...(images?.length ? { images } : {}) },
|
||||
{
|
||||
role: 'user',
|
||||
text: displayText ?? text,
|
||||
...(images?.length ? { images } : {}),
|
||||
...(intoRunningTurn ? { injected: true as const } : {}),
|
||||
},
|
||||
]);
|
||||
setIsGenerating(true);
|
||||
streamingRef.current = '';
|
||||
|
||||
Reference in New Issue
Block a user