mirror of
https://github.com/block/buzz.git
synced 2026-10-30 05:51:46 +00:00
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>
This commit is contained in:
@@ -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 `<img alt="${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();
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 (
|
||||
<MessageItem
|
||||
agentAvatarUrl={props.agentAvatarUrl}
|
||||
agentName={props.agentName}
|
||||
agentPubkey={props.agentPubkey}
|
||||
item={props.item}
|
||||
profiles={props.profiles}
|
||||
/>
|
||||
);
|
||||
return <MessageItem item={props.item} profiles={props.profiles} />;
|
||||
}
|
||||
|
||||
function MessageItem({
|
||||
agentAvatarUrl,
|
||||
agentName,
|
||||
agentPubkey,
|
||||
item,
|
||||
profiles,
|
||||
}: {
|
||||
agentAvatarUrl: string | null;
|
||||
agentName: string;
|
||||
agentPubkey: string;
|
||||
item: Extract<TranscriptItem, { type: "message" }>;
|
||||
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"
|
||||
>
|
||||
<div className="group relative flex w-full min-w-0 flex-col items-start gap-1">
|
||||
{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.
|
||||
<div
|
||||
className="mb-0.5 flex items-center gap-1 text-xs"
|
||||
data-testid="transcript-assistant-identity"
|
||||
>
|
||||
{/*
|
||||
* 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.
|
||||
*/}
|
||||
<span aria-hidden="true" className="flex shrink-0">
|
||||
<UserAvatar
|
||||
avatarUrl={resolvedAgentAvatarUrl}
|
||||
displayName={resolvedAgentName}
|
||||
size="xs"
|
||||
/>
|
||||
</span>
|
||||
<span className="min-w-0 truncate font-normal text-foreground">
|
||||
{resolvedAgentName}
|
||||
</span>
|
||||
</div>
|
||||
) : null}
|
||||
<div
|
||||
className={
|
||||
isCompactPreview
|
||||
|
||||
@@ -393,31 +393,27 @@ async function scrollTranscriptToBottom(page: Page) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Open every collapsed row in the transcript.
|
||||
* Open every folded work block in the transcript.
|
||||
*
|
||||
* The transcript opens with thinking and tool runs folded, so the default frame
|
||||
* is a stack of one-line summaries — true to the product, but it shows none of
|
||||
* the turn's actual shape. Expanding gives the second shot the content the
|
||||
* drawer's width exists for: command output, a failed step, plan items.
|
||||
* The transcript opens with finished work folded, so the default frame is a
|
||||
* stack of one-line summaries—true to the product, but it shows none of the
|
||||
* turn's actual shape. Expanding gives the second shot the content the drawer's
|
||||
* width exists for: command output, a failed step, plan items.
|
||||
*
|
||||
* Driven through the native `<details>` `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
|
||||
* `<details>` 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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user