fix(chat): merge consecutive reasoning steps into one pill#724
Conversation
A multi-step agent turn (reason → tool → reason → tool → answer) produced
a stack of separate "Thought for 1s" pills — one per assistant reasoning
message, with the hidden tool messages between them. This collapses a
reasoning RUN (a maximal sequence of consecutive assistant reasoning
steps separated only by hidden tool messages) into a single pill rendered
at the run's first step: "Thought for {total} · {N} steps", expandable to
the joined reasoning. Single-step turns keep the normal
"Thought for {duration}" pill (label falls back when N == 1).
Adds reasoningRunStart()/reasoningRun() helpers on ChatComponent; uses the
existing chat-reasoning [label] input + resolvedLabel() fallback.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Claude finished @blove's task in 3m 8s —— View job Code Review
SummaryThe approach is sound. Two issues flagged inline: 1. Missing unit tests (moderate) — 2. Everything else checks out:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ? `Thought for ${formatDuration(durationMs ?? 0)} · ${steps.length} steps` | ||
| : undefined; | ||
| return { content, durationMs, streaming, label }; | ||
| } |
There was a problem hiding this comment.
Missing test coverage — reasoningRunStart and reasoningRun are the core of this feature but have no unit tests. The logic has several non-trivial branches worth covering:
- Single reasoning step (no merge) →
labelisundefined,streamingcomes fromisReasoningStreaming. - Two steps separated by a tool message → pill merges both.
- Run ends when a non-reasoning, non-tool message follows.
- All
reasoningDurationMsvalues areundefined→durationMsstaysundefined, label falls back toformatDuration(0)="<1s". - First step is the tail (streaming) vs last step is the tail.
These are good candidates for straight unit tests on the class methods (no template compile needed).
| const label = | ||
| steps.length > 1 | ||
| ? `Thought for ${formatDuration(durationMs ?? 0)} · ${steps.length} steps` | ||
| : undefined; |
There was a problem hiding this comment.
When every step has reasoningDurationMs: undefined, durationMs is undefined here, so durationMs ?? 0 passes 0 to formatDuration, producing "Thought for <1s · N steps" — <1s implies it was fast, but the real meaning is "no timing data". Consider omitting the duration portion when unknown:
| : undefined; | |
| ? steps.length > 1 && durationMs !== undefined | |
| ? `Thought for ${formatDuration(durationMs)} · ${steps.length} steps` | |
| : steps.length > 1 | |
| ? `${steps.length} steps` | |
| : undefined | |
| : undefined; |
Or at minimum document the fallback intent with a comment. (Low severity — timing data is almost always present in practice.)
1 participant
What
A multi-step agent turn (reason → tool → reason → tool → answer) rendered a stack of separate "Thought for 1s" pills — one per assistant reasoning message, with the hidden tool messages between them taking no visible space. The wall of near-identical chips was noisy and pushed the actual answer down.
This collapses a reasoning run — a maximal sequence of consecutive assistant reasoning steps separated only by hidden
toolmessages — into a single pill rendered once at the run's first step:Expanding it shows the joined reasoning from every step. Single-step turns are unchanged: the
[label]falls back to the existingThought for {duration}text whenN == 1.How
chat.component.ts: template renders the reasoning pill only at a run's start (reasoningRunStart(i)), fed by an aggregatedreasoningRun(i)(joins content, sumsreasoningDurationMs, counts steps, derives the streaming flag + merged label).chat-reasoning[label]input and itsresolvedLabel()fallback — no primitive changes.Verification
nx test chatgreen;nx build chatgreen.🤖 Generated with Claude Code