diff --git a/.changeset/fix-empty-thought-dropdown.md b/.changeset/fix-empty-thought-dropdown.md new file mode 100644 index 000000000..f4abb83c5 --- /dev/null +++ b/.changeset/fix-empty-thought-dropdown.md @@ -0,0 +1,5 @@ +--- +"@nanocollective/nanocoder": patch +--- + +Fixed the VS Code extension's thought dropdown expanding to nothing. Streamed tokens are batched behind a 150ms timer, and the reasoning/text routing flag was driven by the `reasoning-start` / `text-start` markers around a batch rather than by the deltas that filled it — so any provider whose ordering differs (the OpenAI Responses API defers `reasoning-end` until the reasoning item completes; openai-compatible providers reopen reasoning without closing text; some emit deltas with no start marker at all) delivered reasoning as assistant text and left the thought view empty. Routing now follows the delta type and flushes the pending batch before switching streams, so a batch always leaves on the callback it was filled for. Whitespace-only reasoning no longer emits an ACP thought chunk, is no longer stored on the message, and no longer opens a thought section, so the empty "Thought for 0s" bubbles are gone. Thanks to @akramcodez. Closes #853. diff --git a/plugins/vscode/media/chat-panel.js b/plugins/vscode/media/chat-panel.js index 2ad8a3c3d..366f788cb 100644 --- a/plugins/vscode/media/chat-panel.js +++ b/plugins/vscode/media/chat-panel.js @@ -1746,13 +1746,14 @@ appendChunk(update.content.text); } } else if (update.sessionUpdate === 'agent_thought_chunk') { - if (!currentThoughtBox) { - endCurrentTextBlock(); - currentThoughtBox = new ThoughtAggregator(); - closeAggregatorIfIdle(); - } - if (update.content && update.content.text) { - currentThoughtBox.append(update.content.text); + const thoughtText = update.content && update.content.text; + if (thoughtText && (currentThoughtBox || thoughtText.trim())) { + if (!currentThoughtBox) { + endCurrentTextBlock(); + currentThoughtBox = new ThoughtAggregator(); + closeAggregatorIfIdle(); + } + currentThoughtBox.append(thoughtText); } } else if (update.sessionUpdate === 'tool_call' || update.sessionUpdate === 'tool_call_update') { if (currentThoughtBox) { diff --git a/source/acp/acp-agent.spec.ts b/source/acp/acp-agent.spec.ts index 9ee80143a..31d865636 100644 --- a/source/acp/acp-agent.spec.ts +++ b/source/acp/acp-agent.spec.ts @@ -244,6 +244,36 @@ test('AcpAgent.loadSession - replays in-memory history for a known session', asy t.true(replayed.some(u => u.update.content.text === 'remember this')); }); +test('AcpAgent.loadSession - replays reasoning but skips whitespace-only reasoning', async t => { + const conn = createMockConn(); + const updates: any[] = []; + conn.sessionUpdate = async (u: any) => { + updates.push(u); + }; + const agent = new AcpAgent(createMockInitContext(), conn); + const session = await agent.newSession({cwd: '/tmp'}); + const loaded = (agent as any).sessions.get(session.sessionId); + loaded.messages = [ + {role: 'assistant', content: 'first', reasoning: '\n\n'}, + {role: 'assistant', content: 'second', reasoning: 'weighing options'}, + ]; + + updates.length = 0; + await agent.loadSession({ + sessionId: session.sessionId, + cwd: '/tmp', + mcpServers: [], + }); + + const thoughts = updates.filter( + u => u.update?.sessionUpdate === 'agent_thought_chunk', + ); + t.deepEqual( + thoughts.map(u => u.update.content.text), + ['weighing options'], + ); +}); + // ============================================================================ // setSessionConfigOption() // ============================================================================ diff --git a/source/acp/acp-agent.ts b/source/acp/acp-agent.ts index 39d27ada3..4b26b8f22 100644 --- a/source/acp/acp-agent.ts +++ b/source/acp/acp-agent.ts @@ -612,7 +612,10 @@ export class AcpAgent implements Agent { }); } } else if (message.role === 'assistant') { - if (message.reasoning && message.reasoning.length > 0) { + // runAcpConversation no longer stores whitespace-only reasoning, so + // this guard is for sessions written before that — replaying one + // would otherwise open a thought section that renders to nothing. + if (message.reasoning && message.reasoning.trim().length > 0) { await this.conn.sessionUpdate({ sessionId: session.sessionId, update: { diff --git a/source/acp/acp-conversation.spec.ts b/source/acp/acp-conversation.spec.ts index 253b81eef..2ae5e39fa 100644 --- a/source/acp/acp-conversation.spec.ts +++ b/source/acp/acp-conversation.spec.ts @@ -256,20 +256,121 @@ test('runAcpConversation - onToken sends agent_message_chunk updates', async t = t.is(messageUpdates[0].update.content.text, 'Hello '); }); +// Streams the reasoning tokens from inside chat(), the way the real client +// does, so the assertions run against a live turn rather than a closure that +// happens to outlive it. +const createReasoningClient = (tokens: string[]): LLMClient => + ({ + chat: async (_msgs: any, _tools: any, callbacks: any) => { + for (const token of tokens) { + callbacks.onReasoningToken(token); + } + return {choices: [{message: {content: 'done'}}]}; + }, + }) as unknown as LLMClient; + +const thoughtTexts = (updates: any[]): string[] => + updates + .filter((u: any) => u.update.sessionUpdate === 'agent_thought_chunk') + .map((u: any) => u.update.content.text); + test('runAcpConversation - onReasoningToken sends agent_thought_chunk updates', async t => { const {conn, updates} = createMockConn(); const session = createMockSession(conn); - let capturedCallbacks: any = null; + + await runAcpConversation({ + session, + client: createReasoningClient(['thinking...']), + toolManager: createMockToolManager() as any, + conn, + nonInteractiveAlwaysAllow: [], + }); + + t.deepEqual(thoughtTexts(updates), ['thinking...']); +}); + +test('runAcpConversation - skips agent_thought_chunk for leading whitespace-only reasoning', async t => { + const {conn, updates} = createMockConn(); + const session = createMockSession(conn); + + await runAcpConversation({ + session, + client: createReasoningClient([ + '\n\n', + 'Analyzing the request', + '\n\n', + 'then answering', + ]), + toolManager: createMockToolManager() as any, + conn, + nonInteractiveAlwaysAllow: [], + }); + + t.deepEqual(thoughtTexts(updates), [ + 'Analyzing the request', + '\n\n', + 'then answering', + ]); +}); + +test('runAcpConversation - stores exactly the reasoning it streamed', async t => { + const {conn, updates} = createMockConn(); + const session = createMockSession(conn); + + await runAcpConversation({ + session, + client: createReasoningClient(['\n\n', ' Weighing', ' options']), + toolManager: createMockToolManager() as any, + conn, + nonInteractiveAlwaysAllow: [], + }); + + // replaySessionHistory re-sends the stored reasoning verbatim, so it has to + // be exactly what the live stream showed - no leading whitespace the thought + // chunks never carried. + const assistant = session.messages.find((m: any) => m.role === 'assistant'); + t.is(assistant?.reasoning, thoughtTexts(updates).join('')); + t.is(assistant?.reasoning, 'Weighing options'); +}); + +test('runAcpConversation - stores no reasoning when it was all whitespace', async t => { + const {conn, updates} = createMockConn(); + const session = createMockSession(conn); + + await runAcpConversation({ + session, + client: createReasoningClient(['\n\n', ' ', '\t']), + toolManager: createMockToolManager() as any, + conn, + nonInteractiveAlwaysAllow: [], + }); + + t.deepEqual(thoughtTexts(updates), []); + const assistant = session.messages.find((m: any) => m.role === 'assistant'); + t.is(assistant?.reasoning, undefined); +}); + +test('runAcpConversation - resets streamed reasoning between turns', async t => { + const {conn} = createMockConn(); + const session = createMockSession(conn); + let turn = 0; const client = { - chat: async ( - _msgs: any, - _tools: any, - callbacks: any, - ) => { - capturedCallbacks = callbacks; - return { - choices: [{message: {content: 'done'}}], - }; + chat: async (_msgs: any, _tools: any, callbacks: any) => { + turn++; + callbacks.onReasoningToken(`turn ${turn} thought`); + if (turn === 1) { + return { + choices: [ + { + message: { + content: '', + tool_calls: [createMockToolCall('read_file', {}, 'call-1')], + }, + }, + ], + }; + } + return {choices: [{message: {content: 'done'}}]}; }, } as unknown as LLMClient; @@ -281,14 +382,10 @@ test('runAcpConversation - onReasoningToken sends agent_thought_chunk updates', nonInteractiveAlwaysAllow: [], }); - t.truthy(capturedCallbacks); - capturedCallbacks.onReasoningToken('thinking...'); - - const thoughtUpdates = updates.filter( - (u: any) => u.update.sessionUpdate === 'agent_thought_chunk', - ); - t.is(thoughtUpdates.length, 1); - t.is(thoughtUpdates[0].update.content.text, 'thinking...'); + const reasonings = session.messages + .filter((m: any) => m.role === 'assistant') + .map((m: any) => m.reasoning); + t.deepEqual(reasonings, ['turn 1 thought', 'turn 2 thought']); }); // ============================================================================ diff --git a/source/acp/acp-conversation.ts b/source/acp/acp-conversation.ts index edc7ff3f2..373b7daaf 100644 --- a/source/acp/acp-conversation.ts +++ b/source/acp/acp-conversation.ts @@ -147,12 +147,22 @@ export async function runAcpConversation( const callbacks: StreamCallbacks = { onReasoningToken: (token: string) => { - streamedReasoning += token; + // Leading whitespace renders to nothing but still opens a thought + // section, leaving a bare "Thought for 0s" bubble, so drop it. + // Only what's emitted is accumulated: replaySessionHistory re-sends + // the stored reasoning verbatim, so anything skipped here has to + // stay out of the message or a reloaded session renders differently + // from the live one. + const text = streamedReasoning ? token : token.trimStart(); + if (!text) { + return; + } + streamedReasoning += text; conn.sessionUpdate({ sessionId: session.sessionId, update: { sessionUpdate: 'agent_thought_chunk', - content: {type: 'text', text: token}, + content: {type: 'text', text}, }, }); }, @@ -237,7 +247,7 @@ export async function runAcpConversation( role: 'assistant', content: cleanedContent, tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined, - reasoning: streamedReasoning || undefined, + reasoning: streamedReasoning.trim() ? streamedReasoning : undefined, }, ]; diff --git a/source/ai-sdk-client/chat/chat-handler.spec.ts b/source/ai-sdk-client/chat/chat-handler.spec.ts index 727221b48..7d3c5d3a7 100644 --- a/source/ai-sdk-client/chat/chat-handler.spec.ts +++ b/source/ai-sdk-client/chat/chat-handler.spec.ts @@ -361,3 +361,161 @@ test('privacy: scrubs outgoing prompts and rehydrates the response at the histor t.is(content, 'Saved real@example.com'); t.false(content.includes('«')); }); + +function streamingModel(parts: Record[]): LanguageModel { + return { + specificationVersion: 'v3', + provider: 'test-provider', + modelId: 'test-model', + doStream: async () => ({ + stream: new ReadableStream({ + start(controller) { + for (const part of parts) { + controller.enqueue(part); + } + controller.enqueue({ + type: 'finish', + finishReason: 'stop', + usage: {inputTokens: 1, outputTokens: 1, totalTokens: 2}, + }); + controller.close(); + }, + }), + }), + } as unknown as LanguageModel; +} + +async function streamRouting(parts: Record[]): Promise<{ + text: string[]; + reasoning: string[]; + content: string; + finalReasoning: string | undefined; +}> { + const text: string[] = []; + const reasoning: string[] = []; + const result = await handleChat({ + model: streamingModel(parts), + currentModel: 'test-model', + providerConfig: { + name: 'TestProvider', + type: 'openai', + models: ['test-model'], + config: {baseURL: 'https://api.test.com'}, + }, + messages: [{role: 'user', content: 'test'}], + tools: {}, + callbacks: { + onToken: token => text.push(token), + onReasoningToken: token => reasoning.push(token), + }, + maxRetries: 0, + }); + return { + text, + reasoning, + content: result.choices[0]?.message.content ?? '', + finalReasoning: result.choices[0]?.message.reasoning, + }; +} + +test('streams reasoning and text to their own callbacks', async t => { + const routed = await streamRouting([ + {type: 'reasoning-start', id: 'r0'}, + {type: 'reasoning-delta', id: 'r0', delta: 'Checking the file'}, + {type: 'reasoning-end', id: 'r0'}, + {type: 'text-start', id: '0'}, + {type: 'text-delta', id: '0', delta: 'Hello'}, + {type: 'text-end', id: '0'}, + ]); + + t.deepEqual(routed.reasoning, ['Checking the file']); + t.deepEqual(routed.text, ['Hello']); + t.is(routed.content, 'Hello'); + t.is(routed.finalReasoning, 'Checking the file'); +}); + +test('buffered reasoning reaches onReasoningToken when text starts without reasoning-end', async t => { + const routed = await streamRouting([ + {type: 'reasoning-start', id: 'r0'}, + {type: 'reasoning-delta', id: 'r0', delta: 'Thinking about it'}, + {type: 'text-start', id: '0'}, + {type: 'text-delta', id: '0', delta: 'Hello'}, + {type: 'text-end', id: '0'}, + ]); + + t.deepEqual(routed.reasoning, ['Thinking about it']); + t.deepEqual(routed.text, ['Hello']); + t.is(routed.content, 'Hello'); +}); + +test('buffered text reaches onToken when reasoning restarts without text-end', async t => { + const routed = await streamRouting([ + {type: 'text-start', id: '0'}, + {type: 'text-delta', id: '0', delta: 'Let me check'}, + {type: 'reasoning-start', id: 'r0'}, + {type: 'reasoning-delta', id: 'r0', delta: 'internal thought'}, + {type: 'reasoning-end', id: 'r0'}, + {type: 'text-delta', id: '0', delta: ' done'}, + {type: 'text-end', id: '0'}, + ]); + + t.deepEqual(routed.text, ['Let me check', ' done']); + t.deepEqual(routed.reasoning, ['internal thought']); + t.is(routed.content, 'Let me check done'); +}); + +test('consecutive reasoning parts stay on the reasoning callback', async t => { + const routed = await streamRouting([ + {type: 'reasoning-start', id: 'r0:0'}, + {type: 'reasoning-delta', id: 'r0:0', delta: 'Part one'}, + {type: 'reasoning-start', id: 'r0:1'}, + {type: 'reasoning-delta', id: 'r0:1', delta: 'Part two'}, + {type: 'text-start', id: '0'}, + {type: 'text-delta', id: '0', delta: 'Answer'}, + {type: 'text-end', id: '0'}, + ]); + + t.deepEqual(routed.reasoning, ['Part one', 'Part two']); + t.deepEqual(routed.text, ['Answer']); +}); + +test('reasoning deltas route on the reasoning callback without a reasoning-start', async t => { + const routed = await streamRouting([ + {type: 'reasoning-delta', id: 'r0', delta: 'Unannounced thought'}, + {type: 'text-start', id: '0'}, + {type: 'text-delta', id: '0', delta: 'Answer'}, + {type: 'text-end', id: '0'}, + ]); + + t.deepEqual(routed.reasoning, ['Unannounced thought']); + t.deepEqual(routed.text, ['Answer']); + t.is(routed.content, 'Answer'); +}); + +test('reasoning deltas after reasoning-end stay on the reasoning callback', async t => { + const routed = await streamRouting([ + {type: 'reasoning-start', id: 'r0'}, + {type: 'reasoning-delta', id: 'r0', delta: 'First half'}, + {type: 'reasoning-end', id: 'r0'}, + {type: 'reasoning-delta', id: 'r0', delta: 'Second half'}, + {type: 'text-start', id: '0'}, + {type: 'text-delta', id: '0', delta: 'Answer'}, + {type: 'text-end', id: '0'}, + ]); + + t.deepEqual(routed.reasoning, ['First half', 'Second half']); + t.deepEqual(routed.text, ['Answer']); + t.is(routed.content, 'Answer'); +}); + +test('alternating deltas with no start markers keep their own callbacks', async t => { + const routed = await streamRouting([ + {type: 'reasoning-delta', id: 'r0', delta: 'Thinking'}, + {type: 'text-delta', id: '0', delta: 'Answer'}, + {type: 'reasoning-delta', id: 'r0', delta: 'More thinking'}, + {type: 'text-delta', id: '0', delta: ' continues'}, + ]); + + t.deepEqual(routed.reasoning, ['Thinking', 'More thinking']); + t.deepEqual(routed.text, ['Answer', ' continues']); +}); diff --git a/source/ai-sdk-client/chat/chat-handler.ts b/source/ai-sdk-client/chat/chat-handler.ts index f6b037f0f..88cc09e85 100644 --- a/source/ai-sdk-client/chat/chat-handler.ts +++ b/source/ai-sdk-client/chat/chat-handler.ts @@ -269,6 +269,12 @@ export async function handleChat( const FLUSH_INTERVAL_MS = 150; let tokenBuffer = ''; let flushTimer: ReturnType | null = null; + // Which callback the buffered run belongs to. Derived from the delta + // that filled the buffer, never from the start/end markers around it — + // providers disagree on those (the Responses API defers reasoning-end + // until the reasoning item completes, openai-compatible reopens + // reasoning without closing text, and some emit deltas with no start + // at all), so routing on them puts reasoning on the text callback. let isReasoning = false; const flushBuffer = () => { @@ -283,10 +289,24 @@ export async function handleChat( flushTimer = null; }; + const flushPending = () => { + if (flushTimer) { + clearTimeout(flushTimer); + } + flushBuffer(); + }; + let lastYield = Date.now(); for await (const chunk of result.fullStream) { switch (chunk.type) { + // The delta type is the source of truth for routing: switching + // streams flushes what the previous one buffered before the flag + // moves, so a batch always leaves on the callback it was filled for. case 'reasoning-delta': + if (!isReasoning) { + flushPending(); + isReasoning = true; + } accumulatedReasoning += chunk.text; tokenBuffer += chunk.text; if (!flushTimer) { @@ -294,6 +314,10 @@ export async function handleChat( } break; case 'text-delta': + if (isReasoning) { + flushPending(); + isReasoning = false; + } accumulatedText += chunk.text; tokenBuffer += chunk.text; if (!flushTimer) { @@ -301,22 +325,13 @@ export async function handleChat( } break; - // Determine which stream to write tokens to + // Pure flush points: they end a batch so each part reaches the UI + // as its own chunk, but they never decide where the next one goes. case 'reasoning-start': - isReasoning = true; - break; case 'text-start': - isReasoning = false; - break; - - // Flush remaining tokens in given stream - case 'text-end': case 'reasoning-end': - if (flushTimer) { - clearTimeout(flushTimer); - } - flushBuffer(); - isReasoning = false; + case 'text-end': + flushPending(); break; } // Periodically yield to the event loop so timers and Ink renders @@ -330,10 +345,7 @@ export async function handleChat( // Safety net: flush any tokens still buffered if the stream ended // without emitting a matching text-end / reasoning-end event. - if (flushTimer) { - clearTimeout(flushTimer); - } - flushBuffer(); + flushPending(); // After streaming completes, collect final results. // `result.usage` is the FINAL step's usage (not `totalUsage`, which diff --git a/source/vscode/chat-panel-thoughts.spec.ts b/source/vscode/chat-panel-thoughts.spec.ts index 3ea6ba8bb..a2ca79a41 100644 --- a/source/vscode/chat-panel-thoughts.spec.ts +++ b/source/vscode/chat-panel-thoughts.spec.ts @@ -210,3 +210,55 @@ test('drops the section when the session is cleared', t => { t.is(boxes.length, 1); t.is(bodyOf(boxes[0]), 'fresh reasoning'); }); + +test('opens no section for whitespace-only reasoning', t => { + const panel = createPanel(); + + panel.thought('\n\n'); + t.is(panel.boxes().length, 0); + + panel.thought(' '); + t.is(panel.boxes().length, 0); + + panel.text('answer'); + panel.finish(); + t.is(panel.boxes().length, 0); +}); + +test('opens the section on the first thought that has content', t => { + const panel = createPanel(); + + panel.thought('\n\n'); + panel.thought('actual reasoning'); + panel.finish(); + + const boxes = panel.boxes(); + t.is(boxes.length, 1); + t.is(bodyOf(boxes[0]), 'actual reasoning'); +}); + +test('keeps appending whitespace once the section is open', t => { + const panel = createPanel(); + + panel.thought('first line'); + panel.thought('\n\n'); + panel.thought('second line'); + panel.finish(); + + t.is(bodyOf(panel.boxes()[0]), 'first line\n\nsecond line'); +}); + +test('an empty thought chunk neither opens a section nor splits the answer', t => { + const panel = createPanel(); + + panel.text('answer '); + const blocks = panel.container.children.length; + + panel.thought(''); + panel.text('continues'); + + // Ending the text block would start a second agent bubble for 'continues', + // so the answer has to still be one child of the container. + t.is(panel.boxes().length, 0); + t.is(panel.container.children.length, blocks); +});