Repository navigation
fix(chat): keep pre-tool answers visible - #1780
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Classify and verify turn content src/lib/workBlocks.ts, src/lib/workBlocks.test.ts, server/claude-turn-boundaries.test.ts, changelog.d/chat-answer-before-tool.md |
splitTurn now places non-commentary text in the answer and commentary text and non-text parts in work. Tests cover answer grouping, work-block open state, and transcript output. The changelog describes the visibility fix. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
devswha/herdr-web-ui#1053: The issue tracks keeping narration between tool runs visible, which aligns with keeping non-commentary text in the main chat.
Suggested reviewers: devswha, demrom
Merge Risk: 
f6d57
-
Answers before tool calls should remain visible when work is collapsed. No issue identified here prevents merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "fix(chat): keep pre-tool answers visible" matches the main change. The changeset stops treating unphased assistant text before tool calls as collapsible work, so those answers stay visible … Linked Issues check Issue #1779requires every non-commentary assistant text segment to remain in the answer, including text before a tool call.splitTurnnow places every non-empty text part except `phase === "comment…Out of Scope Changes check The changes stay within issue #1779. The implementation changes turn classification, the tests verify the required behavior, and the changelog documents the fix. No unrelated behavior or files are int…Description check The description includes the problem context, resulting behavior, change details, linked issue, and validation commands. The required Change and Validation sections are present. Validation results are… Full details: Title check
-
Explanation
The title "fix(chat): keep pre-tool answers visible" matches the main change. The changeset stops treating unphased assistant text before tool calls as collapsible work, so those answers stay visible in the main chat. The title is concise, specific, and follows a conventional commit style.
Full details: Linked Issues check
-
Explanation
Issue
#1779requires every non-commentary assistant text segment to remain in the answer, including text before a tool call.splitTurnnow places every non-empty text part exceptphase === "commentary"inanswer, and keeps commentary and non-text parts inwork. Tests cover unphased text before tools, multi-step answers, closing text, and Claude turn boundaries.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#1779. The implementation changes turn classification, the tests verify the required behavior, and the changelog documents the fix. No unrelated behavior or files are introduced.
Full details: Description check
-
Explanation
The description includes the problem context, resulting behavior, change details, linked issue, and validation commands. The required Change and Validation sections are present. Validation results are explicit for the targeted tests, while typecheck and build are listed without detailed output.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Checked this PR against main Reproduction. On main's code with this PR's two test files, 7 tests fail, including On a merge with main
One CI browser check fails on the merge. Behaviour note. Every unphased assistant text now stays in the main chat and in copied answers, as #1779 asks. Claude leaves its pre-tool status text unphased (only thinking shown as updates is explicit commentary, |
f6d576a to
8e43c4b
Compare
8e43c4b to
cfc8552
Compare
|
Reviewed against main
Only CodeRabbit is currently listed in the PR checks; fork CI still needs maintainer approval.
Review prepared with a code-review agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
|
Merge pass review-work report. Overall: PASSED. This is an outside-contributor PR, and the maintainer approved merging it in this pass. The ext-pr-review lane verified it; see its comment above.
An answer or explanation written before a tool call no longer hides inside the collapsed work block. This is what issue #1779 asks for.
MEDIUM note: for Claude and other agents that don't mark narration as commentary, short status lines between tools now also show below the folded block, and they are included in a copied answer. The maintainer accepted this in an earlier review comment. CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it. |
## Change Chat could show a turn's tool calls above words the agent wrote before them, unlike the terminal. In the reported OmO turn, one assistant message held `thinking | text | toolCall` and two more calls followed. The terminal shows the text first, but Chat showed "Working… · 2 commands" with both commands above that text. Cause: `splitTurn` (`src/lib/workBlocks.ts`) splits a turn into all of its work and all of its answer text, and `Turn` (`src/components/ChatView.tsx`) drew one work block above every answer. Since #1780, unphased text before a tool call stays visible as answer text (not folded into work), so it was drawn below every tool of the turn. The bug is not tied to one harness: Claude, pi/OmO, omp and gjc all write unphased text before tools. Codex's `commentary` text stays inside work, so Codex turns mostly looked right. It showed up only "sometimes" because only turns whose text comes before a tool are affected. Now a turn is drawn as ordered runs (`turnSegments`): each run of work between two answers gets its own work block, so text and tools keep the agent's order, live and after reload. The order follows the Claude app, which interleaves prose with its tool groups. The work-line grouping is unchanged: - A turn with only tools and then an answer still has one `Worked for 7s · N commands` block above the answer. - When a turn has several runs, its last run that draws a row is the head. It reads `Working…`/`Needs you` while the turn runs and carries the turn's duration once settled. The runs before it read `Worked`, so there is still one Working at a time. - Folding is unchanged: blocks are open while live and folded once settled. The final run stays open when commentary comes after the last answer. - Row open state is keyed by the part's index in the turn, so rows in different blocks never share state. No external code was adapted. ## Validation - `server/turn-order.test.ts` parses an OmO/pi transcript with the reported record shape and a Claude turn that talks between tools, then checks the drawn order. `src/lib/workBlocks.test.ts` covers `turnSegments`. On `main` both fail. - `scripts/chat-fallback-browser-qa.ts` (Integration and browser lane) renders the real ChatView. On this branch it shows `["I read this as: …", "work Worked · 3 commands", "The test server …"]`, the Claude turn in five ordered rows, and, with the agent working, only the last run reading `Working…`. On `main` the same script fails with `["work Worked · 3 commands", "I read this as: …", …]`, which is the reported bug. - `bun test src/lib/workBlocks.test.ts server/turn-order.test.ts server/claude-turn-boundaries.test.ts server/skill-activity.test.ts server/codex.test.ts` - `bun run check fast` on 5c9d18d: all steps ok, report verification `verified` (unit: 7665 pass, 0 fail) ## Review fixes (e1f6b47) A running turn with no work yet keeps its Working head. Blocks and rows are named by their parts' indices in the turn, so a run that splits keeps every fold. Todo groups are found once per turn. While the turn runs only the head opens. `src/components/ChatTurn.test.ts` renders `Turn` for these cases; the browser QA adds a run-split check. Details are in the review-resolution comment. --------- Co-authored-by: mp <mp@local>
Context
Gemini 4 Argon (
gemini-4-argon) frequently writes its full multi-paragraph answer in a message withstopReason: "toolUse"and a trailing tool call. It may answer the user's question in full while starting a background status check, then emit only a one-sentence follow-up withstopReason: "stop"after the tool finishes.Codex and Claude usually emit brief status commentary before tool calls and save the full answer for after the final tool call.
splitTurnassumed that ordering, so Argon's substantive pre-tool answer moved into the collapsedWorked for …block once the short follow-up arrived.Change
Classify only explicit commentary as work text. All other assistant text now stays in the main chat, including answer segments before and after tool calls. Reasoning and tool rows remain collapsible.
No external code was adapted.
Validation
bun test src/lib/workBlocks.test.ts server/claude-turn-boundaries.test.ts: 26 passedbun run typecheckbun run buildFixes #1779