From dad000df47998032dbfdde2ce482a28fc0ec7f8c Mon Sep 17 00:00:00 2001 From: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Date: Tue, 25 Aug 2026 18:20:42 -0700 Subject: [PATCH] Remove repeated agent identity from focus answers Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> --- ...TranscriptList.conversationChrome.test.mjs | 144 ++---------------- .../AgentSessionWorkBlock.orphaned.test.mjs | 7 +- .../agents/ui/AgentSessionWorkBlock.tsx | 5 +- .../activityRenderClasses/MessageActivity.tsx | 80 +--------- .../agent-activity-cover-screenshots.spec.ts | 38 +++-- 5 files changed, 36 insertions(+), 238 deletions(-) diff --git a/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationChrome.test.mjs b/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationChrome.test.mjs index 65b28bbab7..9d3085f15e 100644 --- a/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationChrome.test.mjs +++ b/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversationChrome.test.mjs @@ -1,6 +1,6 @@ /** * Presentation contract for the `conversation` transcript variant's *chrome*: - * the agent identity row above each reply, and the focus code-block recipe. + * the single-agent answer treatment and the focus code-block recipe. * * Split out of `AgentSessionTranscriptList.conversation.test.mjs` to stay under * the repo's hard 1000-line/file ceiling (AGENTS.md). The shared jsdom setup, @@ -12,9 +12,6 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { - AGENT, - AGENT_AVATAR_URL, - AGENT_PROFILES, cleanup, fencedCodeItems, fencedCodePromptItems, @@ -22,137 +19,23 @@ import { renderTranscriptWithCodeChrome, } from "./AgentSessionTranscriptList.conversationHarness.mjs"; -test("the conversation identity row announces the agent exactly once", async () => { - // `UserAvatar` names itself — either an `${displayName} avatar` or - // its fallback initials — and the row puts the agent's name in visible text - // immediately after it. Left unhidden, assistive tech reads the same identity - // twice for every single agent turn in the transcript. The visible name is - // the row's one accessible identity; the avatar is decorative. - const { container } = await renderTranscript("conversation", { - agentAvatarUrl: null, - profiles: AGENT_PROFILES, - }); - const identity = container.querySelector( - '[data-testid="transcript-assistant-identity"]', - ); - assert.ok(identity, "the identity row should render"); - - // The avatar is present and still shows the resolved image ... - const decorative = identity.querySelector('[aria-hidden="true"]'); - assert.ok(decorative, "the avatar must be hidden from the accessible tree"); - const image = decorative.querySelector("img"); - assert.ok(image, "hiding the avatar must not stop it rendering visually"); - assert.equal(image.getAttribute("src"), AGENT_AVATAR_URL); - - // ... but every name-bearing node it contains is inside the hidden subtree, - // so the accessible tree is left with exactly one copy of the name. - const named = [...identity.querySelectorAll("img[alt], [aria-label]")]; - assert.ok(named.length > 0, "the avatar should still expose a raw alt/label"); - for (const node of named) { - assert.ok( - node.closest('[aria-hidden="true"]') !== null, - `${node.tagName} carrying an accessible name escapes the hidden avatar subtree`, - ); - } - - // The one remaining accessible identity is the visible name text. - assert.equal(identity.textContent, "Test Agent"); -}); - -test("conversation labels the agent turn with a berd-style identity row", async () => { - // The single biggest divergence from berd was that agent prose carried no - // attribution at all. berd puts a 20px round avatar + the agent name at - // `text-xs` above every reply (MessageBubble.tsx:961-981). +test("conversation keeps assistant prose unboxed without repeating the pane identity", async () => { + // The activity pane is scoped to one agent and its sticky header already + // carries that identity. Repeating avatar + name above every answer adds no + // speaker information and visually collides with message-send tool bubbles. const { container } = await renderTranscript("conversation"); - const identity = container.querySelector( - '[data-testid="transcript-assistant-identity"]', - ); - assert.ok(identity, "conversation should label the agent turn"); - assert.match(identity.textContent, /Test Agent/); - assert.match(identity.className, /text-xs/); - assert.match(identity.className, /gap-1(?!\d)/); - // 20px avatar, berd's size (UserAvatar `size="xs"` → `h-5 w-5`). assert.ok( - identity.querySelector(".h-5.w-5"), - "identity row should carry a 20px avatar", + container.querySelector('[data-testid="transcript-assistant-identity"]') === + null, + "the single-agent pane should not repeat its header identity", ); - // The prose itself stays unboxed and full-width. const message = container.querySelector( '[data-testid="transcript-assistant-message"]', ); + assert.ok(message, "the assistant answer should render"); assert.doesNotMatch(message.innerHTML, /rounded-2xl/); }); -test("the conversation identity row resolves the agent avatar from the profiles lookup", async () => { - // Regression guard for the primary channel flow. `ChannelAgentSessionAgent` - // (useChannelAgentSessions.ts:21-29) has no avatar field at all, so when the - // focus conversation is opened from a channel the panel passes - // `agentAvatarUrl: null` — which is exactly this mount. The row must still - // show the configured avatar by resolving it out of the `profiles` lookup the - // panel already hands down, the same way the ToolItem row - // (ToolItem.tsx:45-52) and the panel header above it already do. Before the - // fix, every channel-opened session fell back to initials. - const { container } = await renderTranscript("conversation", { - agentAvatarUrl: null, - profiles: AGENT_PROFILES, - }); - const identity = container.querySelector( - '[data-testid="transcript-assistant-identity"]', - ); - assert.ok(identity, "the identity row should render"); - const image = identity.querySelector("img"); - assert.ok( - image, - "the profile avatar must win over the caller's null agent record avatar", - ); - assert.equal(image.getAttribute("src"), AGENT_AVATAR_URL); -}); - -test("the conversation identity row resolves the agent name the same way the header does", async () => { - // The row sits directly under the panel header, which labels the same agent - // through `resolveUserLabel` (AgentSessionThreadPanel.tsx:244-249). Reading - // the raw `agentName` prop instead let the two disagree whenever the relay - // profile's display name differed from the caller's agent record. - const { container } = await renderTranscript("conversation", { - agentName: "stale-record-name", - profiles: { - [AGENT.agentPubkey]: { - displayName: "Profile Display Name", - avatarUrl: null, - nip05Handle: null, - ownerPubkey: null, - }, - }, - }); - const identity = container.querySelector( - '[data-testid="transcript-assistant-identity"]', - ); - assert.equal(identity.textContent, "Profile Display Name"); -}); - -test("the conversation identity row keeps the caller's avatar when the lookup has none", async () => { - // The managed-agent path is the other direction: a locally managed agent can - // carry an avatar its relay profile never published. The prop stays the - // fallback, so resolving profile-first must not drop it. - const localAvatar = "https://cdn.example.test/local-managed.png"; - const { container } = await renderTranscript("conversation", { - agentAvatarUrl: localAvatar, - profiles: { - [AGENT.agentPubkey]: { - displayName: "Test Agent", - avatarUrl: null, - nip05Handle: null, - ownerPubkey: null, - }, - }, - }); - const image = container - .querySelector('[data-testid="transcript-assistant-identity"]') - .querySelector("img"); - assert.ok(image, "the caller-supplied avatar should still render"); - assert.equal(image.getAttribute("src"), localAvatar); -}); - test("conversation frames fenced code with berd's header row", async () => { // berd puts the language in a real header row above the frame, with the copy // action opposite it (`code-block.tsx` CodeBlockHeader:388-402), and the code @@ -257,17 +140,14 @@ test("the default transcript variant keeps the legacy code chrome", async () => assert.match(copy.className, /absolute/); }); -test("the identity row is conversation-only", async () => { - // `default`/`compactPreview` markup is pinned byte-for-byte, so the identity - // row must not leak into them. The fixture comparison would catch this too; - // this asserts it directly so the failure names the cause. - for (const variant of ["default", "compactPreview"]) { +test("no transcript variant adds a per-answer identity row", async () => { + for (const variant of ["conversation", "default", "compactPreview"]) { const { container } = await renderTranscript(variant); assert.ok( container.querySelector( '[data-testid="transcript-assistant-identity"]', ) === null, - `${variant} must not render the identity row`, + `${variant} must not render a per-answer identity row`, ); cleanup(); } diff --git a/desktop/src/features/agents/ui/AgentSessionWorkBlock.orphaned.test.mjs b/desktop/src/features/agents/ui/AgentSessionWorkBlock.orphaned.test.mjs index 6bba56be52..23e0e07864 100644 --- a/desktop/src/features/agents/ui/AgentSessionWorkBlock.orphaned.test.mjs +++ b/desktop/src/features/agents/ui/AgentSessionWorkBlock.orphaned.test.mjs @@ -247,10 +247,9 @@ test("thinking renders as a rail row with its own glyph, not a nested disclosure /** * An interim note is progress, not a second reply. * - * #6720 gives every conversation-variant assistant message a 20px avatar + name - * identity row, which is right for the turn's answer. A rail note is the same - * item type, so routing it through that presenter would render a fully - * attributed agent turn nested inside a muted step row — the reader would see + * Conversation mode renders the turn's answer as standalone prose. A rail note + * is the same item type, so routing it through that presenter would render an + * authored agent turn nested inside a muted step row—the reader would see * the agent apparently reply twice, once inside the work it was doing. berd * draws the same line: its `progress` entry is a plain rail row. * diff --git a/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx b/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx index 7f3982deb6..396445b8ac 100644 --- a/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx +++ b/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx @@ -552,9 +552,8 @@ function WorkBlockEntryBody(props: WorkBlockEntryBodyProps) { * chosen to reveal. * * A note deliberately does NOT go through the message presenter. That presenter - * gives every conversation-variant assistant message an avatar + name identity - * row, which is right for the turn's answer and wrong here: a fully attributed - * agent turn nested inside a muted rail step reads as a second reply rather + * renders assistant answers as standalone prose. Sending a note through that + * presenter would still make a muted rail step read as a second reply rather * than as progress. berd draws the same distinction — its `progress` entry is a * plain rail row, not a message bubble. The focus code-block recipe is kept by * providing the same `CodeBlockVariantContext` value the presenter would. diff --git a/desktop/src/features/agents/ui/activityRenderClasses/MessageActivity.tsx b/desktop/src/features/agents/ui/activityRenderClasses/MessageActivity.tsx index 9b1f8d588e..b819050b52 100644 --- a/desktop/src/features/agents/ui/activityRenderClasses/MessageActivity.tsx +++ b/desktop/src/features/agents/ui/activityRenderClasses/MessageActivity.tsx @@ -1,10 +1,5 @@ -import { - resolveUserLabel, - type UserProfileLookup, -} from "@/features/profile/lib/identity"; -import { normalizePubkey } from "@/shared/lib/pubkey"; +import type { UserProfileLookup } from "@/features/profile/lib/identity"; import { Markdown } from "@/shared/ui/markdown"; -import { UserAvatar } from "@/shared/ui/UserAvatar"; import { useAgentSessionTranscriptVariant } from "../agentSessionTranscriptContext"; import { formatTranscriptTimestampTitle } from "../agentSessionUtils"; import type { TranscriptItem } from "../agentSessionTypes"; @@ -21,27 +16,13 @@ export function MessageActivity(props: ActivityRenderClassItemProps) { return null; } - return ( - - ); + return ; } function MessageItem({ - agentAvatarUrl, - agentName, - agentPubkey, item, profiles, }: { - agentAvatarUrl: string | null; - agentName: string; - agentPubkey: string; item: Extract; profiles?: UserProfileLookup; }) { @@ -51,25 +32,6 @@ function MessageItem({ const isAssistant = item.role === "assistant"; const text = item.text.trim(); const messageLink = getTranscriptMessageLink(item); - // The identity row must resolve the agent through the profiles lookup first, - // exactly as `ToolItem` (ToolItem.tsx:45-52) and the panel header - // (AgentSessionThreadPanel.tsx:243-249) already do for the same agent on the - // same surface. The `agentAvatarUrl`/`agentName` props only carry what the - // *caller's* agent record holds, and the primary channel flow's record - // (`ChannelAgentSessionAgent`) has no avatar field at all — so relying on the - // prop alone showed initials for every channel-opened session while the - // managed-agent panel showed the real avatar. Resolving here keeps the row - // agreeing with the header directly above it; the props stay as the fallback - // for callers that have an avatar the lookup does not (a locally managed - // agent whose avatar was never published to a relay profile). - const agentProfile = profiles?.[normalizePubkey(agentPubkey)] ?? null; - const resolvedAgentAvatarUrl = agentProfile?.avatarUrl ?? agentAvatarUrl; - const resolvedAgentName = resolveUserLabel({ - pubkey: agentPubkey, - fallbackName: agentName, - profiles, - preferResolvedSelfLabel: true, - }); if (!isAssistant) { return ( @@ -93,44 +55,6 @@ function MessageItem({ data-testid="transcript-assistant-message" >
- {isConversation ? ( - // berd labels every agent turn with a small identity row above the - // prose — 20px round avatar + name at `text-xs`, `mb-0.5`, `gap-1` - // (MessageBubble.tsx:961-981). Without it the reply reads as - // unattributed body text in a full-cover view, which was the largest - // single divergence from berd. Only the conversation variant gets it: - // the other variants' markup is pinned by the byte-for-byte fixture. -
- {/* - * The avatar is decorative here: `UserAvatar` exposes either an - * image named `${displayName} avatar` or its fallback initials, and - * the agent's name already follows as visible text, so an - * unhidden avatar makes a screen reader announce the same identity - * twice for every agent turn. The visible name is the row's single - * accessible identity. Hidden at this call site rather than by - * teaching the shared `UserAvatar` a decorative mode: other rows - * that pair an avatar with adjacent name text (for example - * `ForumPostCard.tsx:91-99`) have the same shape and would want the - * same treatment, but changing the shared component's accessible - * name affects all 45 of its call sites and is not this PR's scope. - * - * `size="xs"` is already 20px (`h-5 w-5`) in UserAvatar. - */} - - - {resolvedAgentName} - -
- ) : null}
` `open` property rather than by clicking - * a toggle, because the fold's markup and any toggle test id belong to the - * transcript variants being restyled in parallel. `open` is the DOM contract - * underneath whatever they render, and it survives their changes. + * Drive the product's summary buttons rather than reaching through the old + * `
` implementation. This keeps the reference flow exercising the same + * disclosure state a reader uses. */ async function expandTranscriptRows(page: Page) { - const expanded = await page - .getByTestId("agent-session-thread-panel") - .evaluate((panel) => { - const rows = panel.querySelectorAll("details:not([open])"); - for (const row of rows) { - (row as HTMLDetailsElement).open = true; - } - return rows.length; - }); - // A zero here would mean the folds moved and the shot silently became a - // duplicate of the collapsed one, which is the failure worth catching. - expect(expanded).toBeGreaterThan(0); + const summaries = page.getByTestId("transcript-work-block-summary"); + const count = await summaries.count(); + expect(count).toBeGreaterThan(0); + for (let index = 0; index < count; index += 1) { + const summary = summaries.nth(index); + if ((await summary.getAttribute("aria-expanded")) !== "true") { + await summary.click(); + } + } } /**