From 6d37f0876c92b2bc32d36077f0d487e0f027fe8e Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 8 Nov 2026 12:37:57 +0800 Subject: [PATCH] fix(agents): restore the conversation card layout (#2089) * fix(agents): restore the conversation card layout * fix(agents): group consecutive author rows and anchor message footers --- e2e/agentApproval.spec.ts | 39 ++++++++++-- e2e/agentMarkdown.spec.ts | 14 ++--- src/features/agents/Session.module.css | 76 +++++++++++++++-------- src/features/agents/SessionEditor.tsx | 76 +++++++++++------------ src/features/agents/sessionState.test.tsx | 72 ++++++++++++++++++++- 5 files changed, 199 insertions(+), 78 deletions(-) diff --git a/e2e/agentApproval.spec.ts b/e2e/agentApproval.spec.ts index 146aac342..13b10b33f 100644 --- a/e2e/agentApproval.spec.ts +++ b/e2e/agentApproval.spec.ts @@ -23,6 +23,17 @@ const task: AgentTask = { const command = `python3 -c 'import pathlib\np = pathlib.Path("note.txt")\np.write_text("${"long content ".repeat(80)}")'`; const entries: AgentEntry[] = [ { type: "user", text: "Write a note with Python.", attachments: [], steered: false }, + { type: "thought", text: "I need approval to write." }, + { + type: "tool", + id: "inspect", + title: "Read README.md", + kind: "read", + status: "completed", + locations: [], + output: null, + diffs: [], + }, { type: "agent", text: "I will request approval before writing the note." }, { type: "tool", @@ -50,7 +61,7 @@ const entries: AgentEntry[] = [ for (const theme of ["dark", "light"] as const) { for (const width of [760, 1400]) { - test(`approval stays inside a flat transcript at ${width}px in ${theme}`, async ({ page }) => { + test(`approval stays inside the agent column at ${width}px in ${theme}`, async ({ page }) => { await page.setViewportSize({ width, height: 900 }); await page.goto("/"); await page.getByRole("tree", { name: "Project" }).waitFor(); @@ -74,7 +85,7 @@ for (const theme of ["dark", "light"] as const) { await expect(card.locator("pre")).toHaveText(command); await expect(card.getByRole("button")).toHaveText(["Allow Once", "Always Allow", "Decline", "Decline and Stop"]); const geometry = await card.evaluate((el) => { - const transcript = el.parentElement!; + const transcript = el.parentElement!.parentElement!; const prompt = transcript.querySelector('[class*="_user_"]')!; const reply = transcript.querySelector('[class*="_agentCard_"]')!; const rect = el.getBoundingClientRect(); @@ -86,15 +97,31 @@ for (const theme of ["dark", "light"] as const) { promptRadius: parseFloat(getComputedStyle(prompt).borderRadius), replyBackground: getComputedStyle(reply).backgroundColor, replyBorder: getComputedStyle(reply).borderTopWidth, - leftAligned: Math.abs(prompt.getBoundingClientRect().left - reply.getBoundingClientRect().left) < 1, + userOnRight: prompt.getBoundingClientRect().right > reply.getBoundingClientRect().right, + approvalAligned: Math.abs(rect.left - reply.getBoundingClientRect().left) < 1, + authorBeforeWork: + transcript.querySelector('[class*="_agentAuthor_"]')?.nextElementSibling?.matches("details") ?? false, + footerAligned: + Math.abs(prompt.getBoundingClientRect().right - prompt.nextElementSibling!.getBoundingClientRect().right) < + 1, + cardWidth: rect.width, + available: bounds.width, + actionsVisible: getComputedStyle(reply.nextElementSibling!.querySelector('[class*="_messageActions_"]')!) + .opacity, }; }); expect(geometry.scroll).toBe(geometry.client); expect(geometry.contained).toBe(true); expect(geometry.promptRadius).toBeLessThanOrEqual(4); - expect(geometry.replyBackground).toBe("rgba(0, 0, 0, 0)"); - expect(geometry.replyBorder).toBe("0px"); - expect(geometry.leftAligned).toBe(true); + expect(geometry.replyBackground).not.toBe("rgba(0, 0, 0, 0)"); + expect(geometry.replyBorder).toBe("1px"); + expect(geometry.userOnRight).toBe(true); + expect(geometry.approvalAligned).toBe(true); + expect(geometry.authorBeforeWork).toBe(true); + expect(geometry.footerAligned).toBe(true); + // The last message stays actionable even while an approval follows it. + expect(geometry.actionsVisible).toBe("1"); + if (width === 760) expect(geometry.cardWidth).toBeGreaterThan(geometry.available * 0.8); await card.locator("pre").focus(); for (const label of ["Allow Once", "Always Allow", "Decline", "Decline and Stop"]) { await page.keyboard.press("Tab"); diff --git a/e2e/agentMarkdown.spec.ts b/e2e/agentMarkdown.spec.ts index f71476bc1..06ccd9a54 100644 --- a/e2e/agentMarkdown.spec.ts +++ b/e2e/agentMarkdown.spec.ts @@ -165,7 +165,7 @@ test("copies reply Markdown with the keyboard and keeps code copying separate", const copy = page.getByRole("button", { name: "Copy Markdown", exact: true }); const actions = copy.locator(".."); await page.mouse.move(0, 0); - await expect(actions).toHaveCSS("opacity", "0"); + await expect(actions).toHaveCSS("opacity", "1"); await copy.focus(); await expect(actions).toHaveCSS("opacity", "1"); await page.keyboard.press("Enter"); @@ -175,19 +175,19 @@ test("copies reply Markdown with the keyboard and keeps code copying separate", expect(await page.evaluate(() => navigator.clipboard.readText())).toBe("const a = 1;"); }); -test("shows message actions on hover without shifting content and quotes into the draft", async ({ page }) => { +test("keeps latest reply actions visible and quotes into the draft without shifting content", async ({ page }) => { const reply = await openReply(page, REPLY); const copy = page.getByRole("button", { name: "Copy Markdown", exact: true }); const actions = copy.locator(".."); await page.mouse.move(0, 0); - await expect(actions).toHaveCSS("opacity", "0"); + await expect(actions).toHaveCSS("opacity", "1"); const before = await reply.boundingBox(); await reply.hover(); await expect(actions).toHaveCSS("opacity", "1"); expect(await reply.boundingBox()).toEqual(before); await expect(actions.getByRole("button")).toHaveCount(4); await page.mouse.move(0, 0); - await expect(actions).toHaveCSS("opacity", "0"); + await expect(actions).toHaveCSS("opacity", "1"); const userCopy = page.getByRole("button", { name: "Copy", exact: true }); await expect(userCopy.locator("..")).toHaveCSS("opacity", "0"); await page.getByText("Show Markdown", { exact: true }).hover(); @@ -258,8 +258,8 @@ test("keeps chat history readable and selections free of timestamps and status l }); expect(geometry.reply).toBeGreaterThan(0); expect(geometry.reply).toBeLessThan(geometry.root); - expect(geometry.bubbleLeft).toBe(geometry.replyLeft); - expect(geometry.bubbleRight).toBe(geometry.replyRight); + expect(geometry.bubbleLeft).toBeGreaterThan(geometry.replyLeft); + expect(geometry.bubbleRight).toBeGreaterThan(geometry.replyRight); expect(Math.abs(geometry.columnCenter - geometry.composerCenter)).toBeLessThan(1); await stream.getByText("hi", { exact: true }).hover(); await expect(stream.locator("[class*='_messageTime_']").first()).toBeVisible(); @@ -295,7 +295,7 @@ test("keeps chat history readable and selections free of timestamps and status l })); expect(narrow.width).toBeGreaterThan(0); expect(narrow.content).toBeLessThanOrEqual(narrow.width); - expect(narrow.reply).toBe(narrow.width - narrow.padding); + expect(narrow.reply).toBeLessThanOrEqual((narrow.width - narrow.padding) * 0.96); }); test("keeps hover actions reachable when the pointer leaves the message body", async ({ page, context }) => { diff --git a/src/features/agents/Session.module.css b/src/features/agents/Session.module.css index 2876dd52c..f8a1b6b59 100644 --- a/src/features/agents/Session.module.css +++ b/src/features/agents/Session.module.css @@ -1,5 +1,6 @@ .session { - --session-content-width: calc(var(--agent-conversation-width) - var(--space-4) * 6); + --session-content-width: var(--agent-conversation-width); + --session-card-width: 80%; flex: 1; min-height: 0; min-width: 0; @@ -11,6 +12,7 @@ } .conversation { + container-type: inline-size; display: flex; flex-direction: column; min-width: 0; @@ -111,25 +113,19 @@ min-width: 0; width: min(100%, var(--session-content-width)); max-width: 100%; - margin-inline: 0; + margin-inline: auto; box-sizing: border-box; } -/* Messages and folded process rows share the tool-window gutter. */ +.conversationRow { + min-width: 0; +} +/* Keep turns distinct and the work within a turn close to its author. */ .stream > * + * { - margin-top: var(--space-3); + margin-top: var(--space-4); } -.stream > * + .process { +.stream > [data-process] + [data-process] { margin-top: var(--space-2); } -.stream > .process + * { - margin-top: var(--space-2); -} -.stream > * + .agentMessage[data-continuation] { - margin-top: var(--space-2); -} -.stream > .process + .process { - margin-top: 2px; -} .stream:focus { outline: none; } @@ -182,11 +178,19 @@ .messageHead > .role { margin-bottom: 0; } -/* User prompts stay in the same column as replies and tool calls. */ +/* User cards and their footers share a right edge. */ .userMessage { display: flex; flex-direction: column; - align-items: stretch; + align-items: flex-end; +} +.userMessage > .user { + width: fit-content; + max-width: min(var(--session-card-width), 720px); +} +.userMessage > .messageHead, +.userMessage > .messageFooter { + justify-content: flex-end; } /* The actions and time below a message, in flow so they never cover the next entry. */ .messageFooter { @@ -197,9 +201,6 @@ max-width: 100%; flex-wrap: wrap; } -.messageFooter > .messageTime { - order: -1; -} /* Who wrote the message. */ .role { display: flex; @@ -214,14 +215,38 @@ .agentMessage { display: flex; flex-direction: column; - align-items: stretch; + align-items: flex-start; +} +.agentAuthor { + display: flex; + align-items: center; + min-height: var(--h-control-small); +} +.agentAuthor > .role { + margin-bottom: 0; +} +.agentAuthor + * { + margin-top: calc(var(--space-2) / 2); +} +.conversationRow > .process, +.conversationRow > .permissionLine { + max-width: min(var(--session-card-width), calc(var(--session-content-width) * 0.8), 760px); } -.agentMessage > .messageHead, .agentCard { box-sizing: border-box; - width: 100%; + width: fit-content; + max-width: min(var(--session-card-width), calc(var(--session-content-width) * 0.8), 760px); min-width: 0; overflow-wrap: anywhere; + padding: var(--space-2) calc(var(--space-2) + var(--space-2) / 4); + border: 1px solid var(--border); + border-radius: var(--radius-small); + background: var(--bg-elev); +} +@container (max-width: 640px) { + .stream { + --session-card-width: 96%; + } } .messageActions { display: inline-flex; @@ -233,6 +258,7 @@ -webkit-user-select: none; user-select: none; } +.conversationRow:nth-last-child(1 of [data-message]) .messageActions, .message:hover .messageActions, .reply:hover .messageActions, .agentMessage:hover .messageActions, @@ -301,10 +327,10 @@ .user { width: 100%; box-sizing: border-box; - padding: var(--space-2); + padding: var(--space-2) calc(var(--space-2) + var(--space-2) / 4); border: 1px solid var(--border); border-radius: var(--radius-small); - background: var(--bg-elev); + background: var(--bg-tree-selected-inactive); color: var(--fg-strong); line-height: 1.5; white-space: pre-wrap; @@ -792,7 +818,7 @@ display: flex; flex-direction: column; gap: 6px; - align-self: flex-start; + align-self: center; width: 100%; max-width: calc(var(--session-content-width) + var(--space-3) * 2); box-sizing: border-box; diff --git a/src/features/agents/SessionEditor.tsx b/src/features/agents/SessionEditor.tsx index 7b845bbf7..278ee2711 100644 --- a/src/features/agents/SessionEditor.tsx +++ b/src/features/agents/SessionEditor.tsx @@ -603,18 +603,21 @@ function Stream({ task, entries, subagent, onOpenSubagent, finding, findFocus, o const times = useMemo(() => (entries && !subagent ? replyTimes(entries) : null), [entries, subagent]); const busy = subagent ? subagentWorking(subagent.status) : isBusy(task.status); const lastIndex = entries ? entries.length - 1 : -1; + // Keep the row stable when earlier history is prepended; its author can move with the work. + const rows = useMemo(() => (entries ? conversationRows(entries, start) : []), [entries, start]); const authorHeaders = useMemo(() => { const headers = new Set(); - let first = true; - entries?.slice(start).forEach((entry, i) => { - if (entry.type === "user") first = true; - if (entry.type === "agent" && first) { - headers.add(start + i); - first = false; - } + const byAgent = (row: ConversationRow) => + row.kind === "process" || + (row.entry.type !== "user" && + row.entry.type !== "subagent" && + !(row.entry.type === "message" && row.entry.incoming)); + rows.forEach((row, i) => { + const previous = rows[i - 1]; + if (byAgent(row) && (!previous || !byAgent(previous))) headers.add(row.index); }); return headers; - }, [entries, start]); + }, [rows]); const renderRow = useCallback( (row: EntryRow) => ( ), - [task, turns, replies, times, lastIndex, busy, authorHeaders, onOpenSubagent, onDiagramDrawn], + [task, turns, replies, times, lastIndex, busy, onOpenSubagent, onDiagramDrawn], ); - // Only the tail is built while following the end; `start` moves earlier as batches load. - const rows = useMemo(() => (entries ? conversationRows(entries, start) : []), [entries, start]); return (
{finding && } @@ -668,20 +668,30 @@ function Stream({ task, entries, subagent, onOpenSubagent, finding, findFocus, o {entries === null ? ( ) : ( - rows.map((row) => - row.kind === "process" ? ( - - ) : ( - renderRow(row) - ), - ) + rows.map((row) => ( +
+ {authorHeaders.has(row.index) && ( +
+
+ + {providerName(task.provider)} +
+
+ )} + {row.kind === "process" ? ( + + ) : ( + renderRow(row) + )} +
+ )) )} {subagent && subagentWorking(subagent.status) && (
@@ -964,7 +974,6 @@ const Entry = memo(function Entry({ reply, time, working, - showAuthor, onOpenSubagent, onDiagramDrawn, }: { @@ -977,7 +986,6 @@ const Entry = memo(function Entry({ /** When the turn this reply answered was asked and finished. */ time?: ReplyTimes | null; working: boolean; - showAuthor: boolean; onOpenSubagent: (id: string) => void; onDiagramDrawn: () => void; }) { @@ -986,15 +994,7 @@ const Entry = memo(function Entry({ return ; case "agent": return ( -
- {showAuthor && ( -
-
- - {providerName(task.provider)} -
-
- )} +
- +
); diff --git a/src/features/agents/sessionState.test.tsx b/src/features/agents/sessionState.test.tsx index f99573ab6..be2c13f7e 100644 --- a/src/features/agents/sessionState.test.tsx +++ b/src/features/agents/sessionState.test.tsx @@ -121,7 +121,7 @@ beforeEach(() => { }); describe("conversation rows", () => { - it("groups agent authors by turn and uses matching icon-only message action rows", async () => { + it("keeps authors above messages and their folded work with matching icon-only actions", async () => { const user = (text: string): AgentEntry => ({ type: "user", text, @@ -136,12 +136,17 @@ describe("conversation rows", () => { tool("read"), { type: "agent", text: "More detail" }, user("Next question"), + { type: "thought", text: "Considering the next question" }, + tool("next"), { type: "agent", text: "Next reply" }, ]); useAgents.setState({ entries: new Map(backend.entries) }); render(); await screen.findByText("More detail"); - expect(stream().querySelectorAll(`.${styles.agentMessage} .${styles.role}`)).toHaveLength(2); + expect(stream().querySelectorAll(`.${styles.agentAuthor} .${styles.role}`)).toHaveLength(2); + const author = stream().querySelectorAll(`.${styles.agentAuthor}`)[1]!; + expect(author.nextElementSibling).toHaveClass(styles.processRun!); + expect(author.parentElement?.nextElementSibling?.firstElementChild).toHaveClass(styles.agentMessage!); const quote = within(stream()).getAllByRole("button", { name: "Quote Reply" }); const copy = within(stream()).getAllByRole("button", { name: "Copy Markdown" }); expect(quote).toHaveLength(3); @@ -155,6 +160,69 @@ describe("conversation rows", () => { expect(within(stream()).getAllByRole("button", { name: "Copy" })[0]?.className).toBe(copy[0]?.className); }); + it.each([null, "allow"])("keeps tool, reasoning, plan and approval rows in one author run (%s)", async (chosen) => { + backend.entries.set("A", [ + { type: "agent", text: "I’ll read package.json…" }, + tool("one"), + tool("two"), + { type: "agent", text: "More detail" }, + tool("List .", { status: "running" }), + { type: "permission", id: 1, title: "Run python3", options: [], chosen }, + { type: "plan", items: [{ content: "Check files", status: "in_progress" }] }, + { type: "thought", text: "Consider the result" }, + { type: "agent", text: "Done" }, + ]); + useAgents.setState({ entries: new Map(backend.entries) }); + render(); + await screen.findByText("Done"); + expect(stream().querySelectorAll(`.${styles.agentAuthor}`)).toHaveLength(1); + expect(stream().firstElementChild?.firstElementChild).toHaveClass(styles.agentAuthor!); + }); + + it.each([subagent, { type: "message", incoming: true, text: "Parent request" }])( + "starts another author run after $type entries", + async (other) => { + backend.entries.set("A", [ + { type: "agent", text: "Before the switch" }, + other, + { type: "thought", text: "After the switch" }, + tool("read"), + { type: "agent", text: "After the work" }, + ]); + useAgents.setState({ entries: new Map(backend.entries) }); + render(); + await screen.findByText("After the work"); + const authors = stream().querySelectorAll(`.${styles.agentAuthor}`); + expect(authors).toHaveLength(2); + expect(stream().children[1]?.querySelector(`.${styles.agentAuthor}`)).toBeNull(); + expect(authors[1]?.nextElementSibling).toHaveClass(styles.processRun!); + expect(authors[1]?.parentElement?.nextElementSibling?.firstElementChild).toHaveClass(styles.agentMessage!); + }, + ); + + it("anchors every agent footer with its time before the hover actions", async () => { + backend.entries.set("A", [ + { type: "user", text: "Question", attachments: [], steered: false, sentMs: 1000, doneMs: 13000 }, + { type: "agent", text: "First reply" }, + tool("read"), + { type: "agent", text: "Final reply" }, + ]); + useAgents.setState({ entries: new Map(backend.entries) }); + render(); + await screen.findByText("Final reply"); + const messages = stream().querySelectorAll(`.${styles.agentMessage}`); + expect(messages).toHaveLength(2); + for (const message of messages) { + const [card, footer] = message.children; + expect(card).toHaveClass(styles.agentCard!); + expect(footer).toHaveClass(styles.messageFooter!); + expect(footer?.firstElementChild).toHaveClass(styles.messageTime!); + expect(footer?.firstElementChild).toHaveTextContent("12s"); + expect(footer?.lastElementChild).toHaveClass(styles.messageActions!); + } + expect(stream().querySelector(`.${styles.userMessage} > .${styles.messageFooter}`)).not.toBeNull(); + }); + it("builds only the requested tail, with absolute indexes", () => { const entries = Array.from({ length: 45 }, (_, i) => ({ type: "agent", text: `reply ${i}` }) as AgentEntry); const rows = conversationRows(entries, 40);