From ef16743406c3e8c402d7de139890f3de290fd28d Mon Sep 17 00:00:00 2001 From: claude_code Date: Fri, 3 Jul 2026 14:08:16 +0300 Subject: [PATCH] fix(#300 ui): flip avatar hierarchy on comments, make launcher visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two visual defects in the agent avatar stack (PR #304), missed by the code-only review: - The launcher (human) avatar was fully occluded behind the opaque agent glyph — the container was exactly the glyph size, so the launcher sat underneath it. Enlarge the container by an overhang and vertically center the glyph so the launcher peeks out at the bottom-right and stays visible. - On comments the human creator stayed the PRIMARY avatar and name while the stack was crammed into the old badge slot, duplicating the identity and failing the "agent is primary" requirement. AgentAvatarStack gains a showName prop; with showName=false it now replaces the leading avatar for agent comments, and the name slot renders agent.name (+ dimmed · launcher.name). Non-agent comments are byte-identical to before; history-item keeps the default (names shown). Tests: add showName=false and external-MCP (no-launcher) coverage, assert no identity duplication. client tsc clean, 9 tests pass. Co-Authored-By: Claude Opus 4.8 --- .../components/ui/agent-avatar-stack.test.tsx | 18 ++++++ .../src/components/ui/agent-avatar-stack.tsx | 62 +++++++++++++------ .../components/comment-list-item.test.tsx | 32 ++++++++-- .../comment/components/comment-list-item.tsx | 49 ++++++++++----- 4 files changed, 122 insertions(+), 39 deletions(-) diff --git a/apps/client/src/components/ui/agent-avatar-stack.test.tsx b/apps/client/src/components/ui/agent-avatar-stack.test.tsx index 9a6460ab..39d446b1 100644 --- a/apps/client/src/components/ui/agent-avatar-stack.test.tsx +++ b/apps/client/src/components/ui/agent-avatar-stack.test.tsx @@ -43,6 +43,24 @@ describe("AgentAvatarStack", () => { expect(screen.getByText("Alice")).toBeDefined(); }); + it("showName=false: renders only the avatars, no inline name label", () => { + renderStack({ + agent: { name: "Researcher", emoji: "🔬", avatarUrl: null }, + launcher: { name: "Alice", avatarUrl: null }, + aiChatId: "chat-1", + showName: false, + }); + + // The agent glyph is still rendered... + expect(screen.getByText("🔬")).toBeDefined(); + // ...but neither the agent NOR the launcher inline name label is rendered + // (they live only in the hover tooltip, which is not mounted in the initial + // DOM) — guards against suppressing only the agent name and leaking the + // launcher name. + expect(screen.queryByText("Researcher")).toBeNull(); + expect(screen.queryByText("Alice")).toBeNull(); + }); + it("internal chat WITHOUT role: sparkles fallback + 'AI agent' + launcher", () => { const { container } = renderStack({ agent: { name: "AI agent", avatarUrl: null }, diff --git a/apps/client/src/components/ui/agent-avatar-stack.tsx b/apps/client/src/components/ui/agent-avatar-stack.tsx index 430c10c4..59ce252c 100644 --- a/apps/client/src/components/ui/agent-avatar-stack.tsx +++ b/apps/client/src/components/ui/agent-avatar-stack.tsx @@ -27,6 +27,9 @@ export interface LauncherInfo { const AGENT_COLOR = "violet"; const GLYPH_SIZE = 38; const LAUNCHER_SIZE = 22; +// How far the launcher avatar sticks out past the agent's bottom-right corner, so +// the "human behind" reads as behind (lower z-index) yet stays clearly visible. +const LAUNCHER_OVERHANG = 8; /** * The front avatar. Image-source priority (#300): @@ -72,6 +75,9 @@ export interface AgentAvatarStackProps { // (e.g. the page-history row closes the history modal). Keeps this ui/ primitive // free of cross-feature coupling (inherited from the old AiAgentBadge, #143). onActivate?: () => void; + // Whether to render the inline name label next to the avatars (default true). + // Set false when the caller renders the name itself (e.g. the comment row). + showName?: boolean; } /** @@ -87,6 +93,7 @@ export function AgentAvatarStack({ launcher, aiChatId, onActivate, + showName = true, }: AgentAvatarStackProps) { const { t } = useTranslation(); const setAiChatWindowOpen = useSetAtom(aiChatWindowOpenAtom); @@ -117,13 +124,21 @@ export function AgentAvatarStack({ }) : t("AI agent {{name}}", { name: agent.name }); + // The container is only enlarged when there is a launcher to overhang; with no + // human behind it stays tight at the agent glyph size. + const stackSize = launcher ? GLYPH_SIZE + LAUNCHER_OVERHANG : GLYPH_SIZE; + const stack = ( )} - + {/* Pin the agent glyph to the top-left at its own size; the launcher then + overhangs it by LAUNCHER_OVERHANG at the bottom-right and stays visible. */} + @@ -161,21 +185,23 @@ export function AgentAvatarStack({ {stack} - - - {agent.name} - - {launcher && ( - <> - - · - - - {launcher.name} - - - )} - + {showName && ( + + + {agent.name} + + {launcher && ( + <> + + · + + + {launcher.name} + + + )} + + )} ); } diff --git a/apps/client/src/features/comment/components/comment-list-item.test.tsx b/apps/client/src/features/comment/components/comment-list-item.test.tsx index d1034e4a..1826e4ae 100644 --- a/apps/client/src/features/comment/components/comment-list-item.test.tsx +++ b/apps/client/src/features/comment/components/comment-list-item.test.tsx @@ -41,19 +41,39 @@ function renderItem(comment: IComment) { } describe("CommentListItem — agent avatar stack", () => { - it('renders the agent avatar stack when createdSource === "agent"', () => { - // External-MCP shape: agent is the account itself, no launcher behind. + it('flips the hierarchy for an agent comment: agent primary, launcher shown once', () => { + // Internal-chat shape with DISTINCT names so absence-of-duplication is + // assertable: creator is the human "Alice", the acting agent is "Researcher". renderItem( baseComment({ + creator: { id: "user-1", name: "Alice", avatarUrl: null } as any, + createdSource: "agent", + aiChatId: "chat-1", + agent: { name: "Researcher", emoji: "🔬", avatarUrl: null }, + launcher: { name: "Alice", avatarUrl: null }, + }), + ); + // The AGENT is the primary label (the flipped hierarchy). + expect(screen.getByText("Researcher")).toBeDefined(); + // The human launcher name shows exactly once — it is no longer duplicated as + // a separate creator name (that duplication is the bug this fixes). + expect(screen.getAllByText("Alice").length).toBe(1); + }); + + it('external MCP agent comment (no launcher): shows the agent name, no separator', () => { + // aiChatId null => external MCP: the agent IS the account, no human behind. + renderItem( + baseComment({ + creator: { id: "bot-1", name: "MCP Bot", avatarUrl: null } as any, createdSource: "agent", aiChatId: null, - agent: { name: "Service Bot", avatarUrl: null }, + agent: { name: "MCP Bot", avatarUrl: null }, launcher: null, }), ); - // The stack renders the agent name label (the creator name is also shown in - // the row header, so it appears more than once). - expect(screen.getAllByText("Service Bot").length).toBeGreaterThan(0); + expect(screen.getByText("MCP Bot")).toBeDefined(); + // No launcher => no dimmed "·" separator in the header. + expect(screen.queryByText("·")).toBeNull(); }); it('does NOT render the stack for a normal user comment (createdSource "user")', () => { diff --git a/apps/client/src/features/comment/components/comment-list-item.tsx b/apps/client/src/features/comment/components/comment-list-item.tsx index 7167ebdb..072281bb 100644 --- a/apps/client/src/features/comment/components/comment-list-item.tsx +++ b/apps/client/src/features/comment/components/comment-list-item.tsx @@ -119,25 +119,44 @@ function CommentListItem({ return ( - + {comment.createdSource === "agent" && comment.agent ? ( + + ) : ( + + )}
- - {comment.creator.name} - - - {comment.createdSource === "agent" && comment.agent && ( - + {comment.createdSource === "agent" && comment.agent ? ( + <> + + {comment.agent.name} + + {comment.launcher && ( + <> + + · + + + {comment.launcher.name} + + + )} + + ) : ( + + {comment.creator.name} + )}