Skip to content

fix(avatars): scale agent squircles from normalized paths - #7307

Merged
tellaho merged 22 commits into
mainfrom
tho/svg-agent-squircles
Sep 11, 2026
Merged

fix(avatars): scale agent squircles from normalized paths#7307
tellaho merged 22 commits into
mainfrom
tho/svg-agent-squircles

Conversation

@tellaho

@tellaho tellaho commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Overview

Category: fix

User Impact: Agent avatars and empty agent-team placeholders now keep the same intentional squircle silhouette at every size on desktop, while human avatars remain circular.

Problem: Desktop agent avatars used percentage-rounded rectangles, which made the silhouette vary by surface and required each avatar implementation to recreate the shape. Empty team placeholders also drew a regular CSS border inside the clip, leaving straight inset edges that looked like a cropped square. Solution: Reuse one normalized SVG clip path across desktop artwork. Empty team placeholders use a separately clipped squircle outline behind the clipped surface, so both the fill and outline follow the canonical silhouette.

Scope correction: Mobile is deferred from this PR because a real Flutter capture could not be obtained reliably in the current build environment. Previously attached mobile images were hand-composed synthetic layouts, not captures from the running app; they have been removed and must not be treated as validation. The mobile implementation is being reverted in one follow-up commit so it can be reapplied and verified separately.

Related issue

Follow-up to #7106.

Changes

File changes

desktop/src/features/agents/ui/AgentCreationPreview.tsx
Applies the shared squircle clip to agent creation preview images and fallbacks.

desktop/src/features/agents/ui/AgentSessionToolItem/CompactMessageSummary.tsx
Uses the shared agent-avatar silhouette in compact session summaries.

desktop/src/features/agents/ui/IdentityInitialsAvatar.tsx
Clips agent initials with the normalized squircle instead of a percentage radius.

desktop/src/features/agents/ui/TeamIdentityCard.tsx
Uses the shared squircle for agent identities and gives empty teams a separately clipped squircle outline instead of a regular inset border.

desktop/src/features/agents/ui/activityRenderClasses/UserMessageBubble.tsx
Keeps agent avatars in activity message bubbles on the canonical silhouette.

desktop/src/features/home/ui/InboxListPane.tsx
Applies the shared clip to agent avatars in the inbox list.

desktop/src/features/home/ui/InboxMessageRow.tsx
Applies the shared clip to agent avatars in inbox message rows.

desktop/src/features/huddle/components/ParticipantList.tsx
Uses the normalized squircle for agent participants while preserving circular human avatars.

desktop/src/features/messages/ui/MessageRow.tsx
Clips agent message avatars through the shared definition.

desktop/src/features/messages/ui/SystemMessageAvatars.tsx
Uses the canonical agent shape in system-message avatar groups.

desktop/src/features/messages/ui/TypingIndicatorRow.tsx
Uses the canonical agent shape in typing indicators.

desktop/src/features/profile/ui/ProfileAvatar.tsx
Scales the same squircle to the larger agent profile avatar.

desktop/src/features/profile/ui/SelectedRecipientChip.tsx
Uses the shared shape for selected agent recipients.

desktop/src/features/projects/ui/IssueAssigneesRow.tsx
Applies the agent squircle to issue assignees without changing human avatar geometry.

desktop/src/features/projects/ui/ProjectCards.tsx
Uses the canonical shape for agent avatars on project cards.

desktop/src/features/projects/ui/ProjectEntityListRow.tsx
Uses the canonical shape for agent identities in project entity lists.

desktop/src/features/projects/ui/ProjectsActivityFeed.tsx
Uses the shared clip for agent avatars in project activity.

desktop/src/features/projects/ui/ProjectsOverviewRail.tsx
Uses the shared clip for agent avatars in the project overview rail.

desktop/src/main.tsx
Mounts the single shared SVG clip-path definition once for the desktop application.

desktop/src/shared/lib/cn.ts
Registers rounded-squircle in the border-radius class group so shadcn-style cn(...) composition resolves it against rounded-full by normal last-class precedence.

desktop/src/shared/styles/globals/utilities.css
Registers the reusable Tailwind rounded-squircle utility and references the normalized SVG clip path without agent-specific CSS selectors.

desktop/src/shared/ui/AvatarClipPaths.tsx
Owns the reusable normalized object-bounding-box rounded-squircle path so all desktop sizes reuse one definition.

desktop/src/shared/ui/UserAvatar.tsx
Routes the existing squircle shape option through the shared SVG clip.

desktop/tests/e2e/agents.spec.ts
Verifies team facepile and empty-team placeholder surfaces/outlines use the canonical clip without a rectangular border.

desktop/tests/e2e/messaging.spec.ts
Verifies agent message and profile avatars reuse one clip definition and scale across rendered sizes.

Reproduction steps

  1. Run the desktop app and open a channel containing both agent and human messages.
  2. Compare agent avatars in the message list, inbox, huddle participants, projects, and profile panel. Agent images and fallbacks should share one smooth squircle silhouette at each size; human avatars should remain circular.
  3. Open Agents and inspect an empty agent-team card. Its placeholder fill and outline should both be smooth squircles, with no straight inset cropped-square edges.
  4. Open an agent profile from a message and confirm the larger profile avatar keeps the same proportions as the smaller message avatar.

Testing

  • Desktop Biome check passed for the changed empty-team component and Agents E2E spec.
  • Desktop typecheck and differential file-size check passed.
  • Full desktop unit suite passed: 6,453 tests.
  • Full Agents integration spec passed: 37/37, including the empty-team outline regression.
  • Post-rebase pre-push gates passed at 304b159a2: desktop checks, TypeScript typecheck, differential file-size gate, and all 6,453 desktop unit tests.
  • The shadcn-aligned rounded-squircle registration passed changed-file Biome, desktop TypeScript/build, and pre-push desktop checks at 27e0c12d3; the full desktop unit suite passed 6,459/6,459.

Screenshots

Screenshots are temporarily omitted. The previous captures used the desktop E2E mock bridge and were not evidence from a live running app, so they were removed.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 092c6a7277698bd373ccbc1d008fc1507094ae74...636d6bc52811f26de3a7a1e019a8d6807fe1e442.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 636d6bc52811f26de3a7a1e019a8d6807fe1e442 to authorize a new review.
Any previous review applies only to its recorded range.

tellaho pushed a commit that referenced this pull request Sep 3, 2026
@tellaho
tellaho force-pushed the tho/svg-agent-squircles branch from 5bcd234 to 1199f7a Compare September 4, 2026 22:10
@tellaho

tellaho commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Agent-page action-pill masking fix

The visual regression was the legacy avatar cutout being applied behind opaque Start and Restart pills. With squircles, that exposed a conspicuous carved-out hole above the control.

Fixed in 73df83cc4:

  • action pills now overlay an intact squircle avatar with no cutout;
  • availability/status dots still use the squircle-aware cutout for visual separation;
  • the same badge frame stays mounted while Start morphs into the status dot, preserving avatar DOM and animation continuity;
  • the integration regression test now asserts clip-path: none for Start and a polygon clip after the transition to the status dot.

Validation at 73df83cc4:

  • Biome on the three changed files
  • tsc --noEmit
  • all 6,450 desktop unit tests via pre-push hooks
  • targeted integration Playwright test for the Start-pill morph
  • DCO passed; branch range attribution is Taylor Ho with required sign-offs

@tellaho
tellaho force-pushed the tho/svg-agent-squircles branch 2 times, most recently from 990f4ed to 304b159 Compare September 8, 2026 04:44
@tellaho

tellaho commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Empty agent-team placeholder follow-up

The cropped-square artifact on empty team cards came from drawing a regular rectangular CSS border and clipping the same element. The inset border retained straight edges even though the surface was squircle-clipped.

Fixed in 304b159a2:

  • the empty-team placeholder now uses the canonical SVG squircle for its inner surface;
  • a separate ::before layer uses the same clip to draw the outline, so no rectangular border remains;
  • the regression test verifies both clip paths, the absence of a regular border, and that the wrapper itself stays unclipped.

Exact-head validation after rebasing onto origin/main:

  • changed-file Biome check passed;
  • TypeScript typecheck and differential file-size check passed;
  • all 6,453 desktop unit tests passed;
  • full Agents integration spec passed, 37/37;
  • pre-push desktop gates passed at 304b159a2.

@tellaho
tellaho force-pushed the tho/svg-agent-squircles branch from d00b1c4 to 45d57fc Compare September 8, 2026 20:37
@tellaho
tellaho marked this pull request as ready for review September 8, 2026 22:24
@tellaho
tellaho requested a review from a team as a code owner September 8, 2026 22:24

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes required: four P2 regressions/incomplete integration paths

Reviewed head 27e0c12d313e423fc51bf367f32d5b171f5eb42f against base 218633b8fd6ee41aee8eb18ba9806e8d90694751.

The normalized desktop/mobile paths agree, and the selective builtin-artwork migration preserves unmatched custom portraits and linked-instance version drift. The remaining blockers are:

  1. Desktop clips decoration/control shells, hiding speaking/focus indicators and cropping access badges.
  2. Mobile SmallAvatar loses border padding, so opaque avatars paint over facepile separators for humans and agents.
  3. Legacy generated emoji sources are normalized in profile/card rendering but not message/thread rendering.
  4. The new emoji-source normalizer mistakes arbitrary rect+text SVG artwork for a generated emoji and can persist a lossy replacement.

Exit criteria: keep external decoration outside the artwork clip, restore the mobile border inset with a path-aware agent outline, and share strictly recognized legacy-source normalization across avatar consumers without changing authored SVGs. The inline findings identify reproduction paths and targeted regression coverage.

Review scope: source and metadata only; no checkout, builds, tests, imports, or execution of PR code. Read exact-base product/architecture/contributor/testing guidance. Traced shared shape/class composition, images/fallbacks/legacy source, profile/status/action masks, creation/team/empty-team surfaces, message/thread/system/inbox/activity/project/sidebar/huddle consumers, and save/export/native migration contracts. Independent mobile, migration, image-production, and decoration checks were integrated. Author test claims are not independent runtime evidence; live browser/device rendering remains unverified under this automation’s read-only contract. Unchanged relay/CLI/web protocol and authorization paths were excluded. The mobile Huddle-only bot role-classification gap exists at the base and is non-blocking here.

Comment thread desktop/src/features/huddle/components/ParticipantList.tsx Outdated
Comment thread mobile/lib/features/channels/small_avatar.dart Outdated
Comment thread desktop/src/shared/ui/UserAvatar.tsx
Comment thread desktop/src/features/profile/ui/ProfileAvatarEditor.utils.ts

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: 218633b8fd6ee41aee8eb18ba9806e8d90694751..27e0c12d313e423fc51bf367f32d5b171f5eb42f (exact live head 27e0c12d313e423fc51bf367f32d5b171f5eb42f)

Risk: medium — shared avatar shape handling crosses Desktop rendering/accessibility, legacy persisted avatar compatibility, save/export behavior, and Mobile facepile composition.

Four P2 regressions remain. Each was independently traced through its affected consumers and corroborated before this integrated verdict:

  1. Desktop clips interaction and decoration shells. desktop/src/shared/styles/globals/utilities.css:1-5 changes rounded-squircle from radius semantics to a real clip-path, while desktop/src/features/huddle/components/ParticipantList.tsx:374-381,443-459 and desktop/src/features/messages/ui/MessageRow.tsx:456-500,529-537 apply it to outer shells. That clips the huddle speaking halo painted outside the box, outward keyboard focus rings, and the message access-policy badge. A minimal headless-Chromium reproduction at this head removed the external halo and cropped the badge only when the shell carried the clip. Author action: clip only the inner artwork; keep control/decoration layers outside it, render shape-aware halo/outline separately, and add speaking, keyboard-focus, and access-badge regressions. Verification owner: author for coverage, reviewer for browser rendering.

  2. Mobile compact facepiles lose their separator inset. In mobile/lib/features/channels/small_avatar.dart:29-52, replacing the decorated Container with DecoratedBox removes decoration padding. The opaque AvatarImage fills the 20px box and paints over the border beneath it, affecting circular and squircle avatars in overlapping thread/system stacks. Author action: restore an explicit content inset and path-correct agent outline; cover opaque image and fallback cases in mixed human/agent overlap. Verification owner: author for widget coverage, reviewer for rendered widget/device proof.

  3. Legacy generated emoji shape is consumer-dependent. desktop/src/features/profile/ui/ProfileAvatar.tsx:59-64 normalizes an old generated SVG source, but desktop/src/shared/ui/UserAvatar.tsx:61-67 forwards the same stored source unchanged. Upgraded agents can therefore render as a full squircle in profile/card surfaces but retain circular source artwork in message/thread surfaces; the native migration only recognizes builtin portrait fingerprints. Author action: centralize narrowly recognized legacy-source normalization across squircle consumers or complete the stored-profile migration; add a pre-upgrade emoji URL parity regression across profile and message consumers. Verification owner: author for tests, reviewer for browser parity.

  4. Emoji normalization can destructively replace arbitrary authored SVG. desktop/src/features/profile/ui/ProfileAvatarEditor.utils.ts:265-290 recognizes any SVG containing a filled <rect> and <text>, then regenerates a template that discards unrelated paths, styles, dimensions, and positioning. Because URL paste/save reaches this helper, this can persist a lossy replacement rather than merely alter a preview. Author action: rewrite only byte-/structure-equivalent known Buzz templates and preserve every other inline SVG verbatim; test custom rect+text+path artwork and every supported legacy/current generated template through save/export. Verification owner: author for unit and persistence coverage, reviewer for contract recheck.

Validation at matching exact head and clean worktrees:

  • PASS just desktop-test — 6,459/6,459.
  • PASS just desktop-check, just desktop-typecheck, and just desktop-build.
  • PASS independent cd desktop && pnpm test, pnpm typecheck, pnpm build, pnpm build:e2e, and just desktop-tauri-test.
  • PASS just mobile-check; PASS one full just mobile-test run — 2,077/2,077.
  • A second Mobile run reached 2,076/2,077 before cleanup raised PathNotFoundException in voice_note_recording_test.dart; isolated rerun passed 20/20. This is treated as flaky residual confidence, not a PR-caused defect.
  • PASS git diff --check 218633b8fd6ee41aee8eb18ba9806e8d90694751...27e0c12d313e423fc51bf367f32d5b171f5eb42f.
  • Exact-head GitHub Desktop, four smoke shards, integration, macOS/Windows builds, Rust, and Mobile result gates are green. The existing tests do not assert the four failures above.

Manual/native evidence: the three PR screenshots establish idle shape/sizing only. They do not exercise speaking, focus, access badges, legacy emoji parity, or Mobile overlap. A focused browser paint reproduction confirms finding 1. Full native Desktop and rendered-device Mobile proof was not run.

Residual risk: post-fix native/mobile rendered-device verification remains outstanding. The utility name also continues to invite shell clipping by conflating visual radius with destructive paint clipping; boundary discipline plus targeted regression coverage is the smallest safe repair. Any new head requires delta review and targeted proof before clearance.

@tellaho

tellaho commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated update (Carl): the four P2 blockers have been addressed on the branch.

  • 65e9761da reverts the mobile implementation as one isolated commit; the PR now has no mobile diff against its base, so the SmallAvatar separator regression is out of scope for this PR and can be restored/fixed in a follow-up.
  • 39d5d609a centralizes narrowly recognized legacy Buzz emoji normalization across ProfileAvatar and UserAvatar, recognizes only the exact generated SVG template (including both legacy radii), and preserves arbitrary custom inline SVG unchanged.
  • fe985742d keeps clip paths on visual avatar layers rather than interaction/decoration shells, preserves message and huddle focus-ring overflow, and renders the speaking halo as a separately clipped squircle layer.

Validation at exact head fe985742deff1db9f491fbf5273cfd3192fb5878 with a clean tree:

  • focused Node tests: 8/8 passed;
  • Biome on all 11 changed follow-up files passed;
  • focused Playwright messaging avatar regression: 1/1 passed;
  • focused Playwright huddle speaker-ring regression: 1/1 passed;
  • git diff --exit-code 218633b8f...HEAD -- mobile passed (no mobile diff).

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: remaining avatar-button focus clipping

Reviewed exact head fe985742deff1db9f491fbf5273cfd3192fb5878 against base 218633b8fd6ee41aee8eb18ba9806e8d90694751. Rechecked the full two-commit delta from 65e9761d, including 39d5d609 (source normalization) and fe985742 (decoration). This updates the existing review, without duplicate inline findings.

Resolved: legacy-source parity. UserAvatar.tsx:60-69 and ProfileAvatar.tsx:59-65 now use avatarSourceUrlForShape; generated radius-112/radius-256 SVGs normalize consistently for agents, while human sources pass through. The message access badge/focus shell, compact sent-message control, and huddle voice/speaking shells are now unclipped. Mobile remains reverted and is not an acceptance requirement.

  1. [P2] Finish moving focus decoration outside artwork clips. Existing discussion. The repair covers the original message/huddle examples, but the same PR still puts rounded-squircle on focusable buttons that suppress their outline and rely on an outward ring: SystemMessageAvatars.tsx:86-95,147-155, ProjectsActivityFeed.tsx:404-421, and activityRenderClasses/UserMessageBubble.tsx:101-124. utilities.css:1-5 therefore clips the only focus ring for those agent-avatar tab stops. Reproduce by keyboard-focusing an agent system-message avatar, project-activity avatar, or incoming transcript avatar. These buttons already have independently clipped UserAvatar children: keep their outer controls unclipped, as this commit now does for MessageRow. Add a regression on a remaining actual button; the new message/huddle assertions exercise only repaired surfaces.

Non-blocking residual in SVG recognition: the original arbitrary rect/path/text corruption example is now rejected by the full-template guard. A much narrower template-shaped SVG with <tspan fill="red">ACME</tspan> inside its text still matches the (.*?) capture at ProfileAvatarEditor.utils.ts:293; regeneration escapes that child into literal markup. Consider accepting only canonical escaped text and add a text-child fixture. This is a concrete preservation edge case, but its highly constrained input is not an additional merge blocker. Prior SVG discussion.

Stable exit criterion: finish the same unclipped-control repair on the remaining actual buttons and cover one of those remaining production seams. Legacy parity is cleared; no new mobile work or unrelated hardening is requested.

Evidence and limits: source/metadata-only over strict SSH on Wes’s Mac Studio, using pinned raw source and the full prior-to-head diff. Independent decoration and source-normalization lanes were integrated. No checkout, build, tests, browser/native execution, or PR-code imports were performed. Added tests were inspected, not run. Prior whole-feature evidence was reused for unchanged normalized geometry, status/overlap/empty-team composition, and native known-hash migration; unchanged relay/CLI/web authorization and protocol paths remain excluded. This is not a runtime or CI pass.

@tellaho
tellaho force-pushed the tho/svg-agent-squircles branch 2 times, most recently from c1dff0b to 05aa22f Compare September 9, 2026 18:18

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: finish the same focus-shell repair

Reviewed exact head 05aa22f6457db7cd2fbcbcb08cd8b58ae38dc0f6 against base 82656ffea080cc28cec9163ebbf7d1c6be327e43. This updates the prior review and existing focus-clipping discussion, without duplicating inline comments.

Resolved: the previously named SystemMessageAvatars, ProjectsActivityFeed, and incoming transcript activityRenderClasses/UserMessageBubble controls now retain unclipped outer buttons. The new system-message regression asserts that production control is unclipped while its artwork remains clipped. Legacy source parity remains correct, and the exact-template nested <tspan> preservation case is now rejected by [^<>]*. Mobile remains deferred, not an acceptance requirement.

  1. [P2] Keep the remaining Add-avatar and project-people focus rings outside the clip. Two actual buttons in this same feature still combine the rounded-squircle clipping utility with an outward focus ring:

    • AgentCreationPreview.tsx:974-984: the empty Add avatar Popover trigger suppresses its outline and relies on focus-visible:ring-2 / ring-offset-2. AgentDefinitionDialog.tsx:619,751-764 reaches this with a null avatar and no shape override. Reproduce by opening a new agent definition and keyboard-tabbing to Add avatar before choosing artwork.
    • ProjectCards.tsx:85-104: ProjectPeopleStack puts rounded-squircle on the agent’s profile button, despite its UserAvatar child already clipping the artwork. Reproduce by keyboard-tabbing to an agent in a project card’s people stack.

    utilities.css:1-5 clips these buttons, including their outward shadows, and neither button has another visible keyboard-focus treatment. Keyboard users therefore lose the focus indicator at those tab stops. Keep the outer controls unclipped and apply the squircle only to their artwork/inner background, as the repaired controls now do. The changed agents.spec.ts:342-362 only checks the dashed SVG and clicks the trigger; extend coverage to keyboard focus plus an unclipped outer control and add the corresponding project-people seam.

Stable exit criterion: repair these two evidenced focus controls and cover their production seams. This is the same previously identified focus/clip contract, not a request for mobile work, new geometry machinery, or unrelated hardening. Selected-recipient/unassign controls already expose a focus-visible X overlay and are not additional invisible-focus blockers.

Evidence and limits: source/metadata-only over strict SSH on Wes’s Mac Studio. Reviewed the exact current-base feature diff and latest fix commits, with independent decoration and persistence lanes integrated and their material conclusions verified. Shared normalization, legacy generation matching, native asset fingerprints, focus/button ownership, badge/speaking composition, and changed test assertions were inspected. Prior whole-feature evidence was reused for unchanged paths; relay/CLI/web protocol and authorization paths are unchanged and excluded. No checkout, build, tests, browser/native execution, or PR-code imports were performed. This is not a runtime or CI pass.

@tellaho

tellaho commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated update from Carl.

The remaining keyboard-focus blocker is addressed at c3dffbf8d8bad7a9217f1566b7afe1cf46e82ae2: the Add avatar and project-people buttons now remain unclipped, while their inner avatar artwork keeps the canonical SVG squircle clip. Production-path Playwright coverage now tabs to both controls and asserts visible focus decoration, clip-path: none on the button, and squircle clipping on the inner artwork.

Validation at this exact clean head: git diff --check passed; changed-file Biome passed with only the pre-existing unused-helper warning in project-pr-review.spec.ts; push hooks passed desktop check, TypeScript typecheck, file-size validation, and all 6,488 desktop unit tests. The focused built-app Playwright regressions for both controls passed before commit with the same tree.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: one P2 rendering regression

Reviewed head c3dffbf8d8bad7a9217f1566b7afe1cf46e82ae2 against base 82656ffea080cc28cec9163ebbf7d1c6be327e43, with the four-file follow-up checked against previously reviewed 05aa22f6457db7cd2fbcbcb08cd8b58ae38dc0f6. The previously reported Add-avatar and project-people focus clipping is fixed: the interactive shells are now unclipped and their inner artwork retains the squircle clip.

[P2] Keep the idle Plus above the new opaque outline background. In desktop/src/features/agents/ui/AgentCreationPreview.tsx:995-1022, the empty-outline SVG is absolutely positioned and now has opaque bg-background, while the following idle Plus is an unpositioned flex item with only size classes. The positioned SVG paints above that in-flow icon, regardless of their DOM order, so the default empty Add-avatar control loses its visible plus until hover changes the background opacity. This affects the ordinary create-agent preview with no avatar, including the compact rendering. Keep the button unclipped, but explicitly layer the foreground Plus above the outline/background, or move that background into a guaranteed lower layer.

Reproduction to validate: open the create-agent dialog with no avatar and keep the pointer outside Add avatar; inspect the center glyph, then hover the control. This is a source-derived paint-order finding, not a claimed browser reproduction. The governing rules are Flexbox §4.3 and CSS 2.2 Appendix E. The animated upload Spinner is deliberately excluded from this finding because its transform animation changes its stacking behavior.

Nonblocking test note: the new production-control clip-path: none assertions genuinely catch the previous clipping defect. However, agents.spec.ts:355 checking only box-shadow != none also passes with the button’s unconditional shadow-xs; comparing unfocused/focused shadows or checking the expected ring layer would make the additional ring assertion specific.

Scope: source/metadata-only review on Wes’s Studio, with an independent project-people/test and paint-order review. No checkout, build, tests, browser run, or PR-code execution. Unchanged geometry, source normalization, and native migration retain the prior review’s assessment; mobile remains deferred. Exit criterion: retain the repaired focus/artwork boundary while making the idle Plus reliably visible above its background.

Comment thread desktop/src/features/agents/ui/AgentCreationPreview.tsx Outdated

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head c3dffbf8d8bad7a9217f1566b7afe1cf46e82ae2 (base 82656ffea080cc28cec9163ebbf7d1c6be327e43).

One author-actionable P2 remains: rounded-squircle is a real clip-path (desktop/src/shared/styles/globals/utilities.css:1-5), but it is still applied to focusable outer controls in SelectedRecipientChip.tsx:50-58, IssueAssigneesRow.tsx:247-265, and ProjectEntityListRow.tsx:55-77. That clipping region constrains outward keyboard-focus and facepile separator paint, while the avatar child already owns artwork shaping. Keyboard users can therefore lose or receive truncated visible focus indication, and overlap separation can be clipped.

Author action: keep all three control shells unclipped; move squircle clipping to an inner artwork/overlay wrapper, preserve separator paint outside it, and add rendered keyboard-focus regressions for each control plus the overlapped project facepile. Mutation/removal of the corrected boundary must fail behaviorally.

Verification owner: author for implementation/tests; product reviewer for rendered keyboard-focus/overlap verification on the replacement immutable head.

Three prior blockers are cleared: the Mobile separator regression left scope with no Mobile diff; both Desktop avatar consumers now share narrow legacy-template normalization; and strict full-document template recognition preserves custom/nested SVG byte-for-byte. Full Desktop unit suite 6,488/6,488, typecheck, Tauri tests, and git diff --check passed at the exact clean head. Exact-head smoke/build/Rust/security gates were green; Desktop Core was externally cancelled while still progressing, which is a CI confidence gap rather than the basis for this verdict. Native/rendered-device proof remains reviewer-owned confidence after the fix.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: 82656ffea080cc28cec9163ebbf7d1c6be327e43..c3dffbf8d8bad7a9217f1566b7afe1cf46e82ae2 (exact live head c3dffbf8d8bad7a9217f1566b7afe1cf46e82ae2)

Risk: medium — this changes shared avatar clipping across many Desktop interaction, focus, badge, speaking, facepile, legacy-source, save/export, and native migration paths.

Two P2 rendering/accessibility defects remain after integrating independent systems/persistence and product/UI passes:

  1. Focusable outer controls still own the destructive artwork clip. desktop/src/shared/styles/globals/utilities.css:1-5 makes rounded-squircle a real clip-path, but the class remains on profile triggers in desktop/src/features/home/ui/InboxListPane.tsx:388-395 and InboxMessageRow.tsx:202-209, the recipient-removal button in SelectedRecipientChip.tsx:50-58, the unassign button in IssueAssigneesRow.tsx:247-265, and profile-facepile buttons in ProjectEntityListRow.tsx:55-77. The Home controls’ explicit outward focus rings and the project facepile’s separator/focus paint cannot escape those clipped shells. The removal controls expose a focus-state X overlay, but their advertised ring/UA outline remains truncated; that does not make destructive clipping on controls a sound cross-consumer contract. The current regressions cover selected repaired controls, not these production seams. Author action: keep every focusable/control shell unclipped; apply the squircle only to an inner artwork/overlay layer, preserve separator paint outside it, and add causal keyboard-focus coverage for a Home avatar and overlapped project facepile (plus a bounded assertion preventing rounded-squircle from returning to controls). Verification owner: author for implementation/tests; reviewer for rendered keyboard/overlap proof.

  2. The idle Add-avatar Plus is painted behind its new opaque outline layer. In desktop/src/features/agents/ui/AgentCreationPreview.tsx:995-1022, the absolutely positioned outline SVG has bg-background, while the idle Plus is an unpositioned flex item. Positioned descendants with z-index:auto paint above in-flow flex content, so the opaque SVG can hide the ordinary no-avatar Plus until hover changes its background opacity. This affects both regular and compact create-agent previews. Author action: explicitly layer the Plus above the outline/background (or move the background to a guaranteed lower layer) and add a rendered idle-state assertion that fails when the glyph is obscured. Verification owner: author for implementation/test; reviewer for rendered proof.

Prior blockers resolved:

  • Mobile separator inset is removed from this PR by scope reversion: git diff --exit-code 82656ff...c3dffbf -- mobile passes. Mobile normalized rendering is deferred, not validated here.
  • Legacy generated-source parity is shared through avatarSourceUrlForShape in UserAvatar.tsx:72-84 and ProfileAvatar.tsx:70-83; recognized radius-112/256 sources normalize consistently while circle consumers preserve their source.
  • Destructive SVG recognition is now anchored to the exact supported template and rejects nested/custom markup (ProfileAvatarEditor.utils.ts:281-303; preservation fixtures at ProfileAvatarEditor.utils.test.mjs:53-64). Save/export share the narrowly recognized helper.
  • Builtin portrait migration uses exact persona-specific source/media digests and atomic restricted JSON replacement; its tests cover customization preservation, future fields, linked versions, generations, uploaded URLs, and idempotence (migration.rs:549-579,681-708; migration_avatar_tests.rs:4-334). No persistence/migration defect was found.
  • Message access badges and huddle speaking/focus shells inspected in this round retain separate unclipped decoration layers.

Validation at matching clean head:

  • PASS cd desktop && pnpm test — 6,488/6,488.
  • PASS cd desktop && pnpm typecheck.
  • PASS just desktop-tauri-test — all workspace targets green; one declared ignored performance test.
  • PASS git diff --check 82656ffea080cc28cec9163ebbf7d1c6be327e43...c3dffbf8d8bad7a9217f1566b7afe1cf46e82ae2.
  • PASS no-Mobile-diff check above.
  • Exact-head smoke E2E shards, macOS/Windows builds, Rust, DCO, Semgrep, and zizmor are green.

Confidence gaps (not reasons for REQUEST CHANGES): the Desktop Core CI job was externally cancelled while tests were still progressing, so its aggregate is red; CI owns a rerun. No native Desktop interaction capture or mutation run was completed at this head. The reviewed source/tests support the resolved normalization and persistence paths, but do not establish rendered keyboard focus, compact overlap, or idle Add-avatar paint.

Residual risk: Mobile no longer participates in this PR despite the original cross-platform intent. After the two Desktop defects are fixed, exact-head rendered keyboard/paint proof and fresh CI are still required evidence, not additional author defects by themselves.

@tellaho
tellaho force-pushed the tho/svg-agent-squircles branch from c3dffbf to 1e43d06 Compare September 10, 2026 22:36
@tellaho

tellaho commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated update from Carl.

The remaining review blockers are addressed at exact head 1e43d06eb194cc23e829f12a0b478b64ef757ba8:

  • the idle Add-avatar Plus and upload Spinner now sit explicitly above the opaque SVG background (foreground z-10, background z-0);
  • Home inbox avatars, selected-recipient removal chips, issue-assignee controls, and overlapped project facepiles keep destructive squircle clipping on inner artwork only, leaving focus/separator paint on unclipped control shells;
  • production-path Playwright regressions cover idle foreground paint, focused Home/new-DM/assignee controls, and overlapped facepile geometry, including bounded checks that rounded-squircle does not return to those shells.

Validation at this exact clean head after rebasing onto current origin/main: git diff --check passed; push hooks passed Desktop Biome/check, TypeScript typecheck, differential file-size validation, all 6,488 desktop unit tests, and Desktop Tauri clippy/tests. The focused built-app seam set passed 7/7 before commit with the same implementation tree. CI is now running on the pushed exact head.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — APPROVE

Reviewed: 00209076c7a10d9e4a475466c313e8ebecf041f5..1e43d06eb194cc23e829f12a0b478b64ef757ba8 (exact head 1e43d06eb194cc23e829f12a0b478b64ef757ba8)

Risk: medium — shared avatar geometry across many Desktop surfaces, including keyboard-focus shells, status/speaking decoration, facepile overlap, generated-avatar normalization, persistence, and migration. Mobile is explicitly reverted/deferred and absent from the reviewed diff.

Behavior/contracts traced: shared normalized SVG clip ownership; agent squircle versus human circle selection; artwork versus interactive-shell layering; focus/status/separator/speaking paint; empty-team and Add-avatar composition; legacy generated-emoji normalization; managed-agent create/edit persistence and snapshot rasterization; built-in-avatar migration; Desktop call sites and regression seams.

Findings: no unresolved author-actionable defect. The changed head repairs the prior blockers: Add-avatar controls now have explicit outline/content paint order; destructive clipping is removed from focusable/control shells and confined to inner artwork; circle consumers remain circular; speaking/status/overlap decoration remains outside clips; legacy normalization is narrow and shared across rendering/persistence; and Mobile scope is cleanly reverted rather than presented without evidence.

Author action: none for the reviewed code. GitHub currently reports a base conflict; resolve it before merge. Any resulting head SHA expires this approval and requires delta review.

Verification owner: CI owns the remaining exact-head Desktop Core and smoke shards. Reviewer/release QA owns any desired native WebView theme/zoom/accessibility observation and the mandatory post-conflict exact-head refresh.

Validation at matching clean head:

  • Full Desktop JS suite passed independently: 6,493/6,493.
  • Desktop typecheck/check/build and E2E build passed; only existing non-delta diagnostics/chunk warnings were reported.
  • Focused product Playwright passed 10/10 across Add-avatar focus/layering, team overlap and empty outline, status morph, huddle speaking/roster, system-agent focus/decorative stack, normalized message avatar, and issue assignment controls.
  • git diff --check passed.
  • Exact-head macOS/Windows builds, relay-backed Desktop integration, Smoke shards 1 and 3, Windows Rust, lint/security, DCO, Semgrep, and zizmor were green at final refresh. Desktop Core plus Smoke shards 2 and 4 remained pending; no required failure was observed.

Manual/native evidence: six supplied Desktop screenshots were inspected and showed no visible blocker, but they are supporting evidence rather than an independent exact-head native receipt. No fresh native Buzz GUI/WebView or accessibility-tree capture was produced.

Residual risk: native WebView rendering across themes/non-default zoom and assistive technology remains unwitnessed. UserAvatar.tsx retains a shared-to-feature utility import, an existing ownership smell without an established runtime or policy defect. The PR is currently conflicting with the base; conflict resolution is integration work, and this approval must not carry across a changed head.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: preserve the resting project-facepile separator

Reviewed exact head 1e43d06eb194cc23e829f12a0b478b64ef757ba8 against base 00209076c7a10d9e4a475466c313e8ebecf041f5. This updates the prior review. Range-diff confirms the 18 previously reviewed patches are unchanged after rebase; this round adds two repair commits.

Resolved: the Add-avatar Plus and upload spinner now paint explicitly above the opaque SVG background. Home/profile, recipient-removal, assignee-removal and interactive project-facepile controls now keep their focus shells unclipped. Previously cleared generated-source normalization, native migration, badge/status and huddle geometry remain unchanged. Mobile remains deferred and has no base-to-head diff.

[P2] The project facepile’s resting separator is still clipped away. In ProjectEntityListRow.tsx:55-74, the new unclipped trigger owns only the focus ring; ring-2 ring-background remains on the inner UserAvatar. For agents, UserAvatar.tsx:83-95 now applies the real squircle clip to that same element. The outward separator shadow is therefore outside its clipping region. The child span and facepile wrapper supply no alternative resting separator. The noninteractive branch at lines 37-51 has the same ownership issue. At the base, agent avatars used border-radius rather than clip-path, so their outward ring remained visible.

Reproduction to validate: show a project participant stack with two agents, or a human followed by an agent, and leave it unfocused. The later agent overlaps the preceding portrait by -ml-1.5 without the existing two-pixel background separator; adjacent same-color avatars can merge visually. The added project-pr-review.spec.ts:1539-1564 checks shell unclipping and bounding-box overlap, but not the resting separator, so it does not catch this.

Exit criterion: give the resting separator an owner outside the clipped artwork in both facepile branches, preserving the repaired focus shell. Reuse the existing separate-outline pattern if appropriate; no new geometry framework is needed. Cover an unfocused overlap with the agent on top and verify the separator itself, not merely overlap or a computed shadow on a clipped element. This is the remaining separator portion of the existing decoration/clip contract, not a request for mobile work or unrelated hardening.

Evidence and limits: source/metadata-only over strict SSH on Wes’s Studio (BLKD2G9MWNRWY.local, /Users/wesb/dev/buzz object store). Independent persistence, geometry and consumer lanes were integrated; the separator producer/consumer chain and both branches were independently verified. No checkout, build, tests, browser/native run or PR-code execution occurred. This is a source-derived rendering finding, not a claimed visual reproduction. Prior whole-feature evidence was reused for unchanged paths; relay/CLI/web protocol and authorization remain excluded. GitHub also reports a base conflict, which is a separate integration gate rather than the reason for this verdict.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review complete at 1e43d06eb194cc23e829f12a0b478b64ef757ba8 against base 00209076c7a10d9e4a475466c313e8ebecf041f5.

No blocking defects found. The current head repairs the prior clipping/layering failures: normalized squircle clipping is confined to agent artwork while focus, action, status, overlap, and speaking decoration remain outside the clipped layer. Human avatars retain circular rendering. Generated-avatar normalization is consistent through create/edit persistence and PNG snapshot rasterization, while built-in-avatar migration remains fingerprint-bound, custom-avatar preserving, and idempotent. Mobile is explicitly outside this Desktop-scoped diff.

Verification on the exact head:

  • Desktop unit tests: 6,493/6,493 passed.
  • Desktop typecheck, check, production build, and E2E build passed.
  • Focused Playwright coverage passed 10/10 across Add-avatar/focus, team and empty-team outlines, status morphing, huddle speaking/roster, system-agent shells, message-avatar clipping, and issue assign/unassign shells.
  • Full exact-head CI reached terminal state with no failing checks, including Desktop Core, all four Desktop Smoke shards, macOS/Windows builds, relay-backed Desktop integration, Rust lint/Windows, DCO, Semgrep, and zizmor.

Confidence gaps, not defects: a fresh native GUI/theme/non-default-zoom capture was not produced locally because the native sidecar was unavailable; headless assertions and the supplied Desktop screenshots found no issue. UserAvatar.tsx also retains a shared→feature utility import that is architectural debt but not a demonstrated runtime or policy failure.

GitHub reports this head as conflicting with the base. The conflict must be resolved before merge, and any resulting head change invalidates this approval until the delta is reviewed.

tellaho and others added 4 commits September 10, 2026 13:45
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
tellaho and others added 13 commits September 10, 2026 13:45
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho
tellaho force-pushed the tho/svg-agent-squircles branch from 1e43d06 to fe7e03f Compare September 10, 2026 23:54
tellaho pushed a commit that referenced this pull request Sep 10, 2026

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: keep each separator above the preceding portrait

Reviewed exact head fe7e03f204b5b825d134e3ce8ef26a81902d80c0 against base 092c6a7277698bd373ccbc1d008fc1507094ae74. This updates the prior separator finding. The rebase preserves the previous repair patches, apart from a prop-documentation adjustment; the new implementation is the single separator commit.

[P2] The new separator sits behind the neighboring avatar at the overlap. ProjectEntityListRow.tsx:38-40 correctly moves the separator onto an oversized ::before, but its outer shell is only relative inline-flex: it does not establish a stacking context. Both branches then put the artwork into relative z-10 children (lines 50-75). Those children participate in the shared ancestor stacking context, above all the auto/zero-level separator pseudo-elements. The later participant’s separator therefore cannot cover the preceding participant’s portrait where they overlap. The outline is visible around the outside of the pile, but the missing internal separation remains. Human portraits now use this composition too.

Reproduction to validate: leave a project facepile unfocused with an opaque human portrait followed by an opaque agent portrait (the ordering already established in the new test). At -ml-1.5 overlap, inspect the strip immediately left of the later avatar’s artwork: it shows the earlier portrait rather than the intended background-colored separator. This affects both interactive and noninteractive branches. UserProfilePopover forwards the shell classes without adding a per-avatar stacking context; the row-level stacking context contains the entire pile and does not isolate its members.

The governing rule is CSS 2.2 Appendix E.2: positioned z-index:auto descendants do not contain their positive-z-index descendants, which paint later in the parent stacking context. The existing team-avatar implementation already avoids this with a z-index on each outer avatar item (TeamIdentityCard.tsx:170-177).

Smallest repair / stable exit criterion: make each outer separator-plus-artwork shell its own stacking context, for example with isolate in the shared separatorClassName, while retaining visible overflow and the repaired unclipped focus control. Verify that a later separator covers an earlier portrait at their unfocused overlap in both branches. No new geometry machinery, persistence changes or mobile work is needed.

The added test inspects pseudo-element color/insets/opacity and captures a screenshot, but does not assert overlap paint or per-item stacking isolation. Those assertions all pass with the current layering. Bind the regression to the overlap boundary so removing the isolation/order fix fails; a computed pseudo-element background alone cannot prove it is visible above the neighbor.

Scope and evidence: focused source/metadata-only re-review on Wes’s Studio (BLKD2G9MWNRWY.local, /Users/wesb/dev/buzz object store), with independent separator/test review integrated. Exact-base product/contributor/testing documents were confirmed unchanged from the previously read base. Shared avatar and popover consumers, both facepile branches, ancestor stacking, updated tests, and rebase interactions were traced. Earlier Add-avatar/focus, source-normalization, native-migration and badge/huddle assessments stand; mobile has no base-to-head diff. No checkout, build, tests, browser/native run or PR-code execution occurred. The reproduction above is source-derived, not a claimed rendered test result. Unchanged relay/CLI/web protocol and authorization remain excluded. GitHub no longer reports the prior base conflict.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: APPROVE
Reviewed: 092c6a7277698bd373ccbc1d008fc1507094ae74..fe7e03f204b5b825d134e3ce8ef26a81902d80c0 (exact head fe7e03f204b5b825d134e3ce8ef26a81902d80c0)
Risk: medium — visible shared avatar geometry spans persistence/migration, many interactive shells, focus and status decoration, huddle state, facepile overlap, themes, and accessibility.

Findings: no unresolved author-actionable defect. The conflict-resolution rebase preserves the previously reviewed avatar contract, and the one intentional follow-up repairs project facepile separator ownership. Agent artwork owns normalized squircle clipping; interactive shells remain unclipped and focusable; human avatars remain circular; status/speaking/focus decoration stays outside destructive clips. Migration, generated-avatar persistence/export, and shared clip ownership remain coherent.

The complete rebased range was reconciled rather than inheriting approval from the old SHA. Commits 1–19 remained patch-equivalent; conflict resolution preserved both upstream wrapping and the feature’s focus/test behavior. No dropped, duplicated, or base-shadowed behavior was found. The project facepile repair keeps shell focus visible, applies shape-aware separator paint independently, preserves overlap ordering, and covers mixed human/agent output under dark theme and 150% text scale.

Author action: none.
Verification owner: repository merge gates for any future head change; any new head expires this approval.

Exact-head validation: clean fe7e03f204b5b825d134e3ce8ef26a81902d80c0 before and after checks.

  • Desktop unit suite: 6,493/6,493 passed.
  • Tauri package: 3,174 passed, 19 ignored; CSP 7/7 and mixer diagnostic 3/3 passed.
  • Desktop check, typecheck, production build, E2E build, and git diff --check passed.
  • Focused rebuilt Playwright: 4/4 passed across shared clipping, keyboard-focus shell, huddle speaking/control, and mixed project facepile separator behavior. An initial run against a non-E2E build was discarded as harness misuse; the correctly rebuilt run passed.
  • Full Rust workspace passed on rerun; one unrelated PID-parser flake passed independently before the clean rerun.
  • Exact-head CI reached terminal green: Desktop Core, all four smoke shards, macOS/Windows builds, relay-backed integration, Rust lint, DCO, Semgrep, and zizmor passed.
  • All 21 commits carry DCO sign-off; added production code contains no new unsafe, unwrap/expect, debug logging, or undocumented Rust public API.

Manual/native evidence: rebuilt browser-backed screenshot and computed-style/keyboard/ARIA assertions confirm the project facepile seam at dark theme and 150% text scale. No fresh native Tauri/VoiceOver run was performed.

Residual risk: native WebView/VoiceOver rendering across every theme and zoom level was not independently witnessed. Source, rebuilt Playwright, accessibility assertions, screenshot inspection, and terminal exact-head CI found no contradictory evidence.

tellaho pushed a commit that referenced this pull request Sep 11, 2026

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased exact-head review complete at fe7e03f204b5b825d134e3ce8ef26a81902d80c0 against base 092c6a7277698bd373ccbc1d008fc1507094ae74.

No blocking defects found. The complete rebased base..head range and old-to-new mapping were reviewed rather than inheriting the prior approval. Commits 1–19 remain patch-equivalent; the conflict resolution preserves both upstream and PR behavior, with no dropped, duplicated, or base-shadowed change. Migration, generated-avatar persistence/export, and shared clip ownership remain equivalent to the previously reviewed implementation. The new facepile repair keeps clipping and shape-aware separator paint on avatar artwork while leaving interactive shells and focus rings unobstructed; it preserves human-circle and agent-squircle semantics across interactive and static consumers.

Exact-head verification:

  • Desktop unit tests: 6,493/6,493 passed.
  • Full Tauri package: 3,174 passed, 19 ignored; CSP 7/7 and mixer diagnostics 3/3 passed.
  • Desktop typecheck, check, production build, E2E build, and git diff --check passed.
  • Focused rebuilt Playwright coverage passed 4/4 across the normalized shared clip, system-agent keyboard focus shell, huddle speaking/control seam, and mixed human/agent project facepile separator at dark theme and 150% text scale. The screenshot and assertions verify overlap, separator shape, accessible ordered profile controls, unclipped shells, and keyboard focus.
  • Exact-head CI is terminal with no failed checks, including Desktop Core, all four smoke shards, macOS/Windows builds, relay/integration lanes, Rust, DCO, Semgrep, and zizmor.

Non-blocking confidence gap: no fresh native Tauri GUI/VoiceOver run was performed; rebuilt Playwright computed-style, keyboard, ARIA, and screenshot evidence found no contradictory behavior.

Final race check found the PR head/base unchanged and GitHub reports the PR mergeable. Any subsequent head change invalidates this approval until its delta is reviewed.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho

tellaho commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

@wesbillman AI-generated update from Carl: the remaining facepile-stacking blocker is resolved at exact head 636d6bc52811f26de3a7a1e019a8d6807fe1e442.

Each separator-plus-artwork shell now establishes its own stacking context with isolate, in both interactive and static branches, while preserving visible overflow and the unclipped focus shell. The production-seam regression verifies both rendered shells' isolation and samples the actual unfocused overlap pixel against the separator background color; it saves or publishes no screenshot artifact.

Exact-head validation: push gates passed Desktop Biome/check, TypeScript, file-size validation, and 6,493/6,493 Desktop unit tests; rebuilt E2E and the focused Playwright seam passed 1/1; git diff --check passed; the tree is clean and the PR retains zero Mobile diff.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: prior separator blocker resolved

Head: 636d6bc52811f26de3a7a1e019a8d6807fe1e442
Base: 092c6a7277698bd373ccbc1d008fc1507094ae74

No actionable blockers found in this follow-up. The previous separator stacking finding is resolved in source.

  • Production repair: ProjectEntityListRow.tsx:38-40 adds isolate to the shared outer-shell classes used by both noninteractive and interactive branches (:50-76). Each separator and artwork pair now forms its own stacking context, so later pairs paint together above earlier portraits. The separator remains outside the clipped artwork; the focusable shell remains unclipped. This follows the CSS isolation contract.
  • Causal regression coverage: project-pr-review.spec.ts:1578-1579 requires isolation on both human and agent triggers. :1626-1659 additionally samples the separator at the human/agent overlap rather than merely saving a screenshot. Removing the isolation would violate the explicit production-element assertions. Test execution and mutation execution were not performed by this review.
  • Scope and compatibility: this is one commit directly after fe7e03f204b5b825d134e3ce8ef26a81902d80c0, with an unchanged base and only these two changed files. Rechecked shared facepile callers, both interaction branches, image/fallback shape ownership, focus and portalled-popover boundaries. Existing agent-squircle/human-circle contracts remain intact. Earlier reviewed native avatar lifecycle, badge/thread masks, huddle outlines, and other desktop surfaces have no new delta; mobile remains deferred.

Validation: source/spec review on Wes’s Studio (BLKD2G9MWNRWY.local), using exact commit objects in /Users/wesb/dev/buzz. No checkout, build, tests, imports, or PR code execution. Runtime rendering remains unverified. This is a clear review comment, not an approval.

@tellaho

tellaho commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Oops agent decided to assign ya, didn't mean for that 😄

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: APPROVE
Reviewed: 092c6a7277698bd373ccbc1d008fc1507094ae74..636d6bc52811f26de3a7a1e019a8d6807fe1e442 (exact live head 636d6bc52811f26de3a7a1e019a8d6807fe1e442; prior clearance at fe7e03f… was not carried forward)
Risk: medium — shared avatar rendering plus compatibility migration/persistence/export behavior.

Behavior/contracts traced: normalized squircle registration and root-mounted object-bounding-box clip ownership; UserAvatar/ProfileAvatar and profile, huddle, thread, message/inbox, project-facepile, and assignee consumers; preservation of human circles and unclipped focus shells; canonical emoji normalization versus custom/nested SVG handling; snapshot export; built-in-avatar migration and atomic JSON persistence; changed-head facepile stacking/separator behavior; packaging and changed-surface boundaries.

Findings: no blocking or non-blocking author-actionable defect found. The migration updates only exact known built-in generations, preserves customized/drifted versions, supports multiple generations, and updates synced source versions. Independent hashing matched the three predecessor portrait data URLs to the registered migration hashes. The changed-head isolate is correctly scoped to the facepile separator shell. Its production-rendered dark-theme pixel assertion was mutation-proved: removing production isolation and direct class assertions failed 3/3 attempts ([26,26,26] expected, [244,185,66] rendered), then passed after restoring the exact head. Human avatar paths remain circular, while agent artwork receives the normalized squircle clip; interactive focus decoration remains outside the clip.

Author action: none.
Verification owner: reviewer/tooling owns the remaining native WKWebView/AX, retained screenshot, light-theme, and zoom confidence gaps; CI/release gates own artifact/platform freshness.

Validation at matching HEAD:

  • just desktop-test: PASS, 6,493/6,493.
  • pnpm build:e2e plus focused production-bound smoke journey: PASS, 1/1.
  • pnpm lint: PASS, no errors (only unrelated existing warnings/infos).
  • just desktop-tauri-test: migration coverage PASS; package total 3,172 passed / 2 failed / 19 ignored. Both failures are unrelated pre-existing process-discovery PID-fixture failures in bounded_command.rs, outside the 42-file PR diff.
  • git diff --check 092c6a7...HEAD: PASS; all 21 commits have DCO sign-off.
  • Exact-head GitHub checks are green for Desktop Core, four smoke E2E shards, integration E2E, macOS and Windows builds, Desktop Release Candidate, Rust lint, DCO, Semgrep, and zizmor. GitHub reports MERGEABLE.
  • Final reviewer checkouts matched remote head 636d6bc52811f26de3a7a1e019a8d6807fe1e442, were detached and clean, and had the exact expected merge-base.

Manual/native evidence: production-bound browser pixels, keyboard focus, role/name semantics, agent/human shape distinction, and geometry were exercised. No native WKWebView/AX run or retained reviewer screenshot was produced.

Residual risk: WebKit-specific clip paint, light theme, and zoom were not independently witnessed. This is a reviewer evidence gap rather than author rework; causal production-pixel coverage, full renderer tests, platform builds, and clean source tracing materially reduce the risk.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: APPROVE
Reviewed: 092c6a7277698bd373ccbc1d008fc1507094ae74..636d6bc52811f26de3a7a1e019a8d6807fe1e442 (exact live head; prior approval at fe7e03f… expired)
Risk: high — avatar geometry crosses persisted built-ins, export/rasterization, shared paint/focus ownership, and many production identity surfaces.

No author-actionable defect remains in the complete 42-file range or the changed-head delta.

The migration updates only exact known built-in avatar generations, preserves customizations and drifted versions, updates synced instance source versions, and retains restricted atomic JSON persistence (desktop/src-tauri/src/migration.rs:527-567,594-713; desktop/src-tauri/src/migration_avatar_tests.rs:3-240). Independent hashes of the predecessor portrait data URLs match the registered migration hashes. Persisted custom/nested SVG remains unchanged or rejected by the strict canonical parser; canonical emoji is normalized only for squircle consumers, and snapshot rasterization uses the same normalizer (desktop/src/features/profile/ProfileAvatarEditor.utils.ts:256-304; desktop/src/features/agents/managedAgentAvatar.ts:25-33; desktop/src/shared/avatar/snapshotAvatarPng.ts:54-72).

Clip ownership is centralized in one object-bounding-box definition mounted once at the app root (desktop/src/shared/avatar/AvatarClipPaths.tsx:1-99; desktop/src/main.tsx:14-15,85-88). Agent artwork receives the squircle while interactive shells remain unclipped for focus paint; reviewed human paths remain circular. The changed-head isolate in ProjectEntityListRow.tsx:39 establishes the facepile separator stacking context. Removing production isolation while also removing its direct class assertions still makes the pixel assertion fail 3/3, proving the dark-theme paint seam causally.

Author action: none.
Verification owner: reviewer/tooling for optional native WKWebView/AX, light-theme, and zoom observation; unrelated local Tauri PID-fixture failures remain tooling-owned.

Exact-head validation

  • Full Desktop unit suite: 6,493/6,493 passed.
  • Focused production smoke journey: 1/1 passed after pnpm build:e2e.
  • pnpm lint: passed with unrelated existing warnings/infos; git diff --check: passed.
  • Tauri migration rows: passed. Full Tauri package: 3,172 passed / 2 failed / 19 ignored; both failures are unrelated process-discovery PID fixtures outside the 42-file diff.
  • Exact-head GitHub Desktop Core, smoke shards, relay/integration E2E, macOS/Windows builds, release candidate, Rust lint, DCO, Semgrep, and zizmor are green.
  • No native media was captured. Dark-theme production pixels, keyboard focus, role/name semantics, and agent/human shape distinction were exercised in browser E2E.
  • Both lane trees ended clean at the exact live head.

Any new head invalidates this approval.

@tellaho
tellaho merged commit ec11f8e into main Sep 11, 2026
61 checks passed
@tellaho
tellaho deleted the tho/svg-agent-squircles branch September 11, 2026 16:08
rileycrane pushed a commit that referenced this pull request Sep 12, 2026
* origin/main:
  Configure ACP session scope per agent (#7578)
  refactor(buzz-acp): point agents at buzz --help instead of a command table (#7586)
  feat(buzz-cli): render an agent-friendly command tree in --help (#7584)
  fix(avatars): scale agent squircles from normalized paths (#7307)
  fix(mobile): bind same-name mentions to exact selected identities (#7385)
  fix(desktop): isolate quota backoff and reuse channel discovery rosters (#6998)
  test(desktop): isolate login-shell probe measurements (#7570)
  feat(git): add default-branch management to relay and CLI (#7562)
  fix(acp): integrate the Buzz Pi adapter fork (#7552)
  fix(markdown): align mention chip wrapping (#7501)
  fix(relay): reject presence updates when Redis storage fails (#7532)
  fix(desktop): let inbox title and message author names truncate under narrow panes (#7550)
  fix(buzz-acp): report missing models without retrying (#7538)
  fix(desktop): require a Codex adapter with Astra support (#7427)
  fix(desktop): order unnamed roster members by full canonical npub (#7503)
  fix(mobile): standardize public-key identity display on npub (#7493)
  fix(desktop): npub identity controls across profile, agents, and workflows (#7489)
  fix(desktop): npub identity displays for mention, member, and workflow surfaces (#7495)
  fix(desktop): shared npub identity foundation (canonicalNpub, PubKey gate, strict parser) (#7488)
  fix(mobile): render push notification sender identity as npub (#7494)

Signed-off-by: Sol <478bb5a31222ea2b28a3d1afb8b1d598940628f19c2a87efc3c4b822299eeec6@buzz.block.builderlab.xyz>

# Conflicts:
#	desktop/src-tauri/src/commands/media_download.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants