From cfc8552f61d2aa75f533cff7869dab6265b7191a Mon Sep 17 00:00:00 2001 From: Oded Falik <33235305+odfalik@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:21:59 -0700 Subject: [PATCH] fix(chat): keep pre-tool answers visible --- changelog.d/chat-answer-before-tool.md | 3 +++ scripts/chat-fallback-browser-qa.ts | 11 +++++++---- server/claude-turn-boundaries.test.ts | 13 +++++++++---- src/lib/workBlocks.test.ts | 27 ++++++++++++++++++-------- src/lib/workBlocks.ts | 22 ++++++++------------- 5 files changed, 46 insertions(+), 30 deletions(-) create mode 100644 changelog.d/chat-answer-before-tool.md diff --git a/changelog.d/chat-answer-before-tool.md b/changelog.d/chat-answer-before-tool.md new file mode 100644 index 0000000000..d986198d64 --- /dev/null +++ b/changelog.d/chat-answer-before-tool.md @@ -0,0 +1,3 @@ +### Fixed +- An assistant answer no longer disappears inside the collapsed work summary when it is followed by + a tool call or another answer. diff --git a/scripts/chat-fallback-browser-qa.ts b/scripts/chat-fallback-browser-qa.ts index 4d375d17d5..ede0864e87 100644 --- a/scripts/chat-fallback-browser-qa.ts +++ b/scripts/chat-fallback-browser-qa.ts @@ -109,7 +109,7 @@ try { await page.getByText("No conversation yet — say something below", { exact: true }).waitFor(); assert.equal(await page.locator(".chat-terminal-fallback").count(), 0, "an empty structured conversation keeps its ordinary prompt"); // Native parser output crosses HTTP into the real ChatView. Later activity must not - // move a final reply into the fold or duplicate it; narration and tools stay there. + // move a final reply into the fold or duplicate it. Only explicit commentary joins the tools there. const jsonl = (...records: unknown[]) => records.map((record) => JSON.stringify(record)).join("\n"); source = "claude-transcript"; turns = parseClaudeTranscript(jsonl( @@ -149,7 +149,8 @@ try { )); await page.evaluate(() => { window.chatFallbackQA.agent("claude"); window.chatFallbackQA.pane("agent"); window.chatFallbackQA.refresh(); }); await page.getByText(answer, { exact: true }).waitFor(); - const work = page.locator(".work-block"); + const continuation = page.locator(".chat-turn-agent").nth(1); + const work = continuation.locator(".work-block"); const head = work.locator(".work-block-head"); await head.waitFor(); if (await head.getAttribute("aria-expanded") === "true") await head.click(); @@ -157,15 +158,17 @@ try { assert.equal(await work.getByText(answer, { exact: true }).count(), 0, "the reply is outside work"); assert.equal(await page.getByText(answer, { exact: true }).count(), 1, "the reply appears once"); assert.equal(await page.getByText(answer, { exact: true }).isVisible(), true, "folding work keeps the reply visible"); + assert.equal(await continuation.getByText(narration, { exact: true }).isVisible(), true, "unphased narration stays visible outside folded work"); + assert.equal(await work.getByText(narration, { exact: true }).count(), 0, "unphased narration is not part of work"); + assert.equal(await page.getByText(narration, { exact: true }).count(), 1, "unphased narration appears once"); assert.equal(await page.getByText("Catalog verification complete.", { exact: true }).isVisible(), true, "the hook continuation's short reply is also visible"); assert.equal(await page.locator(".chat-turn-agent").count(), 2, "hook continuation answers have separate bubbles"); assert.equal(await page.getByText(/Stop hook feedback:/).count(), 0, "injected hook context is not a user message"); await head.click(); - await work.getByText(narration, { exact: true }).waitFor(); assert.equal(await work.locator(".work-row-head").count(), 1, "the tool stays inside work"); await head.click(); if (process.env.UI_EVIDENCE_DIR) await page.screenshot({ path: join(process.env.UI_EVIDENCE_DIR, "claude-hook-separate-replies.png") }); - console.log("PASS Claude hook replies stay separately visible outside folded work; narration and tool remain grouped"); + console.log("PASS Claude hook narration and replies stay visible outside folded work; the tool remains inside"); } assert.deepEqual(errors, []); console.log("chat fallback: browser OK (no hidden terminal reads, opened stalled output cannot block structured refresh, late cleanup, non-agent inline output, fallback error announcement and accurate empty state)"); diff --git a/server/claude-turn-boundaries.test.ts b/server/claude-turn-boundaries.test.ts index 99df75758a..9f729f3dda 100644 --- a/server/claude-turn-boundaries.test.ts +++ b/server/claude-turn-boundaries.test.ts @@ -24,8 +24,11 @@ describe("Claude completed turns", () => { reply("Verified.", "2026-01-01T12:02:00Z"), )); expect(turns.map((turn) => turn.role)).toEqual(["user", "assistant", "assistant"]); - expect(turns.slice(1).map((turn) => splitTurn(turn.parts).answer.map((part) => part.text))).toEqual([["The library has three books.", "Two are available."], ["Verified."]]); - expect(turns.slice(1).map((turn) => splitTurn(turn.parts).work.map((part) => part.kind === "text" ? part.text : part.kind))).toEqual([["Checking the catalog.", "tool"], ["Verifying the catalog.", "tool"]]); + expect(turns.slice(1).map((turn) => splitTurn(turn.parts).answer.map((part) => part.text))).toEqual([ + ["Checking the catalog.", "The library has three books.", "Two are available."], + ["Verifying the catalog.", "Verified."], + ]); + expect(turns.slice(1).map((turn) => splitTurn(turn.parts).work.map((part) => part.kind))).toEqual([["tool"], ["tool"]]); expect(turns.slice(1).flatMap((turn) => turn.parts.filter((part) => part.kind === "tool").map((part) => part.output))).toEqual(["Three books", "Three books confirmed"]); }); } @@ -41,9 +44,11 @@ describe("Claude completed turns", () => { reply("The background check timed out.", "2026-01-01T12:03:00Z"), )); expect(turns.map((turn) => turn.role)).toEqual(["assistant", "assistant", "assistant"]); - expect(turns.map((turn) => splitTurn(turn.parts).answer.map((part) => part.text))).toEqual([["The catalog has three books."], ["Two books are available."], ["The background check timed out."]]); + expect(turns.map((turn) => splitTurn(turn.parts).answer.map((part) => part.text))).toEqual([ + ["The catalog has three books."], ["Checking availability.", "Two books are available."], ["The background check timed out."], + ]); expect(turns.map((turn) => turn.end_ts)).toEqual(["2026-01-01T12:00:00Z", "2026-01-01T12:02:00Z", "2026-01-01T12:03:00Z"]); - expect(splitTurn(turns[1]!.parts).work.map((part) => part.kind === "text" ? part.text : part.kind)).toEqual(["Checking availability.", "tool"]); + expect(splitTurn(turns[1]!.parts).work.map((part) => part.kind)).toEqual(["tool"]); expect(splitTurn(turns[1]!.parts).work.find((part) => part.kind === "tool")).toMatchObject({ output: "Two available" }); }); }); diff --git a/src/lib/workBlocks.test.ts b/src/lib/workBlocks.test.ts index 4064df169d..a2e121e2ac 100644 --- a/src/lib/workBlocks.test.ts +++ b/src/lib/workBlocks.test.ts @@ -8,9 +8,10 @@ const text = (value: string): Extract => ({ const thinking = (value: string): Extract => ({ kind: "thinking", text: value }); describe("splitTurn", () => { - it("folds every action and the narration between them into the work; the trailing prose is the answer", () => { - const split = splitTurn([thinking("hm"), tool("read"), text("looking…"), tool("edit"), text("done"), text("really")]); - expect(split.work).toEqual([thinking("hm"), tool("read"), text("looking…"), tool("edit")]); + it("folds actions and explicit narration into work while every assistant response stays in the answer", () => { + const narration = { ...text("looking…"), phase: "commentary" as const }; + const split = splitTurn([thinking("hm"), tool("read"), narration, tool("edit"), text("done"), text("really")]); + expect(split.work).toEqual([thinking("hm"), tool("read"), narration, tool("edit")]); expect(split.answer).toEqual([text("done"), text("really")]); }); @@ -18,8 +19,8 @@ describe("splitTurn", () => { expect(splitTurn([text(" \n"), text("hi")])).toEqual({ work: [], answer: [text("hi")] }); }); - it("is all work when the turn ends on an action (still running)", () => { - expect(splitTurn([text("on it"), tool("bash")])).toEqual({ work: [text("on it"), tool("bash")], answer: [] }); + it("keeps an unphased response visible when a tool follows it", () => { + expect(splitTurn([text("The substantive answer"), tool("bash")])).toEqual({ work: [tool("bash")], answer: [text("The substantive answer")] }); }); it("keeps Codex commentary inside work even after the last tool or with no tools", () => { @@ -32,6 +33,16 @@ describe("splitTurn", () => { const final = { ...text("Done"), phase: "final_answer" as const }; expect(splitTurn([tool("exec_command"), final, tool("cleanup")])).toEqual({ work: [tool("exec_command"), tool("cleanup")], answer: [final] }); }); + + it("keeps a multi-step answer and later closing sentence outside the work block", () => { + const answer = text("The report contains three sections.\n\n1. Summary.\n2. Details.\n3. Next steps."); + const check = { ...tool("bash"), summary: "bun test report" }; + const closing = text("The report is ready."); + const parts = [answer, check, closing]; + expect(splitTurn(parts)).toEqual({ work: [check], answer: [answer, closing] }); + expect(workStartsOpen(true, parts)).toBe(true); + expect(workStartsOpen(false, parts)).toBe(false); + }); }); describe("workStartsOpen", () => { @@ -50,10 +61,10 @@ describe("workStartsOpen", () => { expect(workStartsOpen(false, [commentary("Checking."), tool("exec_command"), final("Done"), tool("cleanup")])).toBe(false); }); - it("keeps a settled turn open when its words are followed by an action and no answer: the fold would hide all of them", () => { - expect(workStartsOpen(false, [text("on it"), tool("bash")])).toBe(true); + it("folds a settled turn when an unphased answer is followed by an action because the answer remains visible", () => { + expect(workStartsOpen(false, [text("The answer"), tool("bash")])).toBe(false); // a todo update after the closing summary is not even counted in the header - expect(workStartsOpen(false, [tool("read"), text("Here is the full answer."), tool("TodoWrite")])).toBe(true); + expect(workStartsOpen(false, [tool("read"), text("Here is the full answer."), tool("TodoWrite")])).toBe(false); }); it("keeps a settled turn open when it ends in Codex commentary: folding would hide its last text", () => { diff --git a/src/lib/workBlocks.ts b/src/lib/workBlocks.ts index b57ddff2fd..4a55e1c8cf 100644 --- a/src/lib/workBlocks.ts +++ b/src/lib/workBlocks.ts @@ -8,9 +8,9 @@ export type ThinkingPart = Extract; export type TextPart = Extract; /** - * A turn the way Codex shows it: everything the agent did on the way — tool calls, - * reasoning and the narration between them — folded under one "Worked for 7s · 1 edit" - * header, and only what it said after the last action left out in the open as the answer. + * A turn the way Codex shows it: tool calls, reasoning and explicit progress narration fold under + * one "Worked for 7s · 1 edit" header. Every other assistant response stays in the main chat, + * including an unphased response followed by more work or another answer. */ export interface SplitTurn { work: ConversationPart[]; @@ -18,15 +18,10 @@ export interface SplitTurn { } export function splitTurn(parts: ConversationPart[]): SplitTurn { - let lastAction = -1; - for (let index = 0; index < parts.length; index += 1) { - const part = parts[index]; - if (part?.kind !== "text" || part.phase === "commentary") lastAction = index; - } const isProse = (part: ConversationPart): part is TextPart => part.kind === "text" && part.text.trim().length > 0; return { - work: parts.filter((part, index) => part.kind !== "text" || (isProse(part) && part.phase !== "final_answer" && index <= lastAction)), - answer: parts.filter((part, index): part is TextPart => isProse(part) && (part.phase === "final_answer" || index > lastAction)), + work: parts.filter((part) => part.kind !== "text" || (isProse(part) && part.phase === "commentary")), + answer: parts.filter((part): part is TextPart => isProse(part) && part.phase !== "commentary"), }; } @@ -34,10 +29,9 @@ export function splitTurn(parts: ConversationPart[]): SplitTurn { * A work block is open while its turn runs and folded once the turn settles: the answer stays * outside the fold, so a finished turn reads as its answer under one "Worked for" row. * - * A settled turn keeps its block open when the work holds words the answer does not end on: - * prose with no answer at all (an action came after the last words, or Codex commentary had no - * final answer), or prose recorded after the last answer part. Folding would hide the newest - * thing the agent said, or all of it. It takes the turn's parts, not the split: the order decides. + * A settled turn keeps its block open when explicit commentary comes after the last answer, or + * when commentary has no answer at all. Folding would hide the newest thing the agent said, or all + * of it. It takes the turn's parts, not the split: the order decides. */ export function workStartsOpen(live: boolean, parts: ConversationPart[]): boolean { if (live) return true;