fix(chat): render completed reasoning in full instead of simulating streaming (#2736)
fix(chat): render completed reasoning in full instead of simulating streaming
This commit is contained in:
@@ -89,6 +89,7 @@ Use this doc when you ask an agent to change tool/header/description behavior.
|
||||
- The rich tool diff preview lives in `ToolPartDiffPreview.tsx` and is lazy-loaded from `ToolPart`. It is the only tool-card piece that imports the `@pierre/diffs` + Shiki rendering stack, keeping that stack out of the eager chat startup graph. While its chunk loads (first rendered diff only) the plain-text patch from `PlainDiffFallback.tsx` renders as the Suspense fallback, mirroring the preview's error fallback. `ToolPart` itself must not statically import `@pierre/diffs` runtime modules or `@/lib/shiki/appThemeRegistry`.
|
||||
- Running bash output falls back to `state.metadata.output` until canonical `state.output` arrives. Its output viewport grows with the content up to `46vh`, then scrolls and follows new output until the user scrolls up; following resumes when the user returns to the bottom. Live output appends or replaces rewritten snapshots as plain text without worker highlighting; finalized output normalizes ANSI terminal controls with a bounded synthetic-cell budget, bypasses the throttle, and receives the normal one-time highlighted rendering.
|
||||
- Thinking/Justification duration is hidden in `sorted` mode (handled in `ReasoningPart.tsx` + `JustificationBlock.tsx`).
|
||||
- Reasoning streaming presentation derives from the live stream phase (`streaming`/`cooldown`), never from missing persisted timing: a cached part without `time.end` is not live, and a part whose `time.end` is set never streams (issue #2020).
|
||||
|
||||
## "I want to change description for Perplexity" (example recipe)
|
||||
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import React from 'react';
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { renderToStaticMarkup } from 'react-dom/server';
|
||||
import type { Part } from '@opencode-ai/sdk/v2';
|
||||
|
||||
import { I18nProvider } from '@/lib/i18n';
|
||||
import { ReasoningTimelineBlock } from './ReasoningPart';
|
||||
import ReasoningPart, { ReasoningTimelineBlock } from './ReasoningPart';
|
||||
import type { StreamPhase } from '../types';
|
||||
|
||||
// A reasoning text whose summary (first 120 chars) fits in the header but
|
||||
// whose expanded body content should only appear when the disclosure is open.
|
||||
@@ -113,3 +115,80 @@ describe('ReasoningTimelineBlock', () => {
|
||||
expect(markup).not.toContain('<!-- -->');
|
||||
});
|
||||
});
|
||||
|
||||
// Regression tests for issue #2020: a persisted reasoning part must not be
|
||||
// presented as live streaming just because cached data lacks `time.end` or a
|
||||
// stream phase. Live activity derives from the live stream phase only.
|
||||
describe('ReasoningPart streaming gating (issue #2020)', () => {
|
||||
// Short enough (< 80 chars) that the collapsed header summary contains the
|
||||
// complete text, letting us assert full content on first paint.
|
||||
const SHORT_REASONING = 'Persisted reasoning text that is already fully available.';
|
||||
|
||||
const BUSY_INDICATOR = 'animate-busy-pulse';
|
||||
|
||||
const makeReasoningPart = (time?: { start?: number; end?: number }): Part =>
|
||||
({
|
||||
id: 'prt_reasoning_2020',
|
||||
sessionID: 'ses_2020',
|
||||
messageID: 'msg_2020',
|
||||
type: 'reasoning',
|
||||
text: SHORT_REASONING,
|
||||
time,
|
||||
}) as unknown as Part;
|
||||
|
||||
// Server rendering reads the UI store's initial state, which is
|
||||
// chatRenderMode 'live' — the mode in which the streaming presentation is
|
||||
// reachable and the issue reproduces.
|
||||
const renderPart = (part: Part, streamPhase?: StreamPhase): string =>
|
||||
renderToStaticMarkup(
|
||||
<I18nProvider>
|
||||
<ReasoningPart part={part} messageId="msg_2020" streamPhase={streamPhase} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
|
||||
test('reasoning without time.end and without a live stream phase renders complete, not streaming', () => {
|
||||
// Freshly opened completed session: cached part never received `time.end`
|
||||
// and no message-level stream phase is available. The full text is already
|
||||
// local, so the block must render as finished content on first paint.
|
||||
const markup = renderPart(makeReasoningPart({ start: 1_000 }), undefined);
|
||||
|
||||
expect(markup).not.toContain(BUSY_INDICATOR);
|
||||
expect(markup).toContain('aria-expanded="false"');
|
||||
expect(markup).toContain(SHORT_REASONING);
|
||||
});
|
||||
|
||||
test('reasoning without time.end in a completed message renders complete, not streaming', () => {
|
||||
const markup = renderPart(makeReasoningPart({ start: 1_000 }), 'completed');
|
||||
|
||||
expect(markup).not.toContain(BUSY_INDICATOR);
|
||||
expect(markup).toContain('aria-expanded="false"');
|
||||
expect(markup).toContain(SHORT_REASONING);
|
||||
});
|
||||
|
||||
test('reasoning with time.end is never treated as streaming, even when the phase claims streaming', () => {
|
||||
const markup = renderPart(makeReasoningPart({ start: 1_000, end: 2_000 }), 'streaming');
|
||||
|
||||
expect(markup).not.toContain(BUSY_INDICATOR);
|
||||
expect(markup).toContain('aria-expanded="false"');
|
||||
expect(markup).toContain(SHORT_REASONING);
|
||||
});
|
||||
|
||||
test('live in-progress reasoning still renders as streaming', () => {
|
||||
// Genuinely live: the message-level stream phase reports streaming and the
|
||||
// part has not ended. The block auto-expands and shows the busy indicator.
|
||||
const markup = renderPart(makeReasoningPart({ start: 1_000 }), 'streaming');
|
||||
|
||||
expect(markup).toContain(BUSY_INDICATOR);
|
||||
expect(markup).toContain('aria-expanded="true"');
|
||||
});
|
||||
|
||||
test('remounting a completed reasoning part does not re-trigger the streaming presentation', () => {
|
||||
const part = makeReasoningPart({ start: 1_000 });
|
||||
const first = renderPart(part, undefined);
|
||||
const second = renderPart(part, undefined);
|
||||
|
||||
expect(second).toBe(first);
|
||||
expect(second).not.toContain(BUSY_INDICATOR);
|
||||
expect(second).toContain(SHORT_REASONING);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -261,7 +261,11 @@ export const ReasoningTimelineBlock: React.FC<ReasoningTimelineBlockProps> = ({
|
||||
};
|
||||
}, []);
|
||||
|
||||
if (!text || text.trim().length === 0) {
|
||||
// While genuinely streaming, the busy header must appear as soon as
|
||||
// reasoning starts even before the block-level reveal (commitStreamedText)
|
||||
// has committed a first complete line — otherwise "Thinking…" never shows
|
||||
// for the first moments of a short, single-paragraph response.
|
||||
if (!isStreaming && (!text || text.trim().length === 0)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -430,8 +434,12 @@ const ReasoningPart = React.memo(({
|
||||
const rawText = partWithText.text || partWithText.content || '';
|
||||
const textContent = React.useMemo(() => cleanReasoningText(rawText), [rawText]);
|
||||
const time = partWithText.time;
|
||||
const canBeStreaming = streamPhase === undefined || streamPhase !== 'completed';
|
||||
const isStreaming = chatRenderMode === 'live' && canBeStreaming && typeof time?.end !== 'number';
|
||||
// Live activity derives from the live stream phase, never from the absence
|
||||
// of persisted timing data: cached parts may lack `time.end` even though
|
||||
// the message finished long ago (issue #2020). A part that has ended is
|
||||
// never streaming, even while the rest of the message still streams.
|
||||
const isLiveStreamPhase = streamPhase === 'streaming' || streamPhase === 'cooldown';
|
||||
const isStreaming = chatRenderMode === 'live' && isLiveStreamPhase && typeof time?.end !== 'number';
|
||||
const throttledTextRaw = useStreamingTextThrottle({
|
||||
text: textContent,
|
||||
isStreaming,
|
||||
@@ -441,9 +449,11 @@ const ReasoningPart = React.memo(({
|
||||
// never mutates in place.
|
||||
const throttledText = isStreaming ? commitStreamedText(throttledTextRaw) : throttledTextRaw;
|
||||
|
||||
// Show reasoning even if time.end isn't set yet (during streaming)
|
||||
// Only hide if there's no text content
|
||||
if (!throttledText || throttledText.trim().length === 0) {
|
||||
// Show reasoning even if time.end isn't set yet (during streaming).
|
||||
// While genuinely streaming, keep the block mounted even before the
|
||||
// block-level reveal commits a first line, so the busy header appears
|
||||
// immediately instead of waiting on committed text.
|
||||
if (!isStreaming && (!throttledText || throttledText.trim().length === 0)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user