feat: badge-linked profile avatars (TASK-22142) - #2929
Conversation
Twenty basic avatars plus three per badge for Bug Whisperer and Offramp User, drawn for avatar size in the badge house style. badge-assets.json now carries the avatar slugs from the API catalog, so the UI can build the picker and assert the files exist without keeping its own list of badge codes.
…ial (TASK-22142) The generated dot face is gone: it was not launch-ready and drew on name strings. UserAvatar renders the user's picked character on its palette, or exactly one character of the username on the username's color — never the full name or verification data. The dead UserHeader component goes with it.
… to it (TASK-22142) Two groups — what the user's badges unlocked, then the basics — a tap saves at once, the die randomizes across the whole pool for free, and a badge that ships avatars says so in the earn toast with a Choose avatar control that deep-links to /profile?avatarPicker=true (nuqs URL state). Fixture routes may now carry their own query so ds-shots can capture the open picker.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…he generated face The dot-face avatar (497ab2a) reads as a random emoji on the home chip and the profile header. Regress to the initials avatar in a first-letter mode for the user's own identity; contacts keep two-letter initials. The face is parked until avatar v2 — recover it from 497ab2a. Claude-Session: https://claude.ai/code/session_01BQuSkygKUt9jbs2N4iCKtk
Code-analysis diffPainscore total: 7063.56 → 7097.14 (+33.58) 🆕 New findings (48)
…and 28 more. ✅ Resolved (37)
…and 17 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 6 screens moved12 of 60 shots changed · 48 identical · baseline
new screens (2)
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
…142) The one-letter own-avatar now comes from AvatarWithBadge's firstLetterOnly mode (#2924, cherry-picked underneath). UserAvatar only adds the picked-key branch and otherwise renders that same fallback, so the fallback lives in one place. HomeTopNav and the self ProfileHeader pass the pick through.
The picked-avatar circle used an inline style for its triple; the ratchet counts inline styles and only lets them go down. Emit the seven avatar triples as full class literals instead, like the rest of the app.
Three avatars per badge (prod holder counts, 2026-09-02), drawn for the circle in the house style; SUPPORT_SURVIVOR reuses the Bug Whisperer set. Manifest regenerated from the API catalog.
Support Survivor shared Bug Whisperer's three avatars, so the 14 users who hold both saw six identical tiles. Bandage, headset and beer are its own (API bbec8dd4 declares the same slugs).
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — changes requested
One major finding remains: P1 is still present, while P2 is fixed at this head.
Findings
-
MAJOR · src/components/Avatar/AvatarPicker.tsx:61 · Serialize avatar saves so the last tap wins
Tap avatar A and then B before the first request settles. Both updates run concurrently; if A completes after B, the server ends on A, and each completion also refreshes the user, so the older refresh can overwrite the optimistic B selection. A failed older request can likewise roll the UI back after B succeeds. Serialize or coalesce avatar mutations and only roll back or refresh the request that is still current; add a deferred-promise test proving the final tap wins when completions reverse. -
BLOCKING · src/types/badge-assets.json:97 · [claude-opus] SUPPORT_SURVIVOR avatar slugs disagree with the API's generated manifest
src/types/badge-assets.json declares"SUPPORT_SURVIVOR": ["beetle", "shell", "peek"](line 97), and the art shipped in this PR matches it: public/avatars/badge/SUPPORT_SURVIVOR/{beetle,shell,peek}.svg. The other half of this change, peanut-api-ts#1498, declares the opposite — BADGE_CATALOG gives SUPPORT_SURVIVORavatars: ['bandage', 'headset', 'beer'], and its generated docs/badge-assets.json carries the same three, with an explicit comment that this set must NOT be a copy of Bug Whisperer's because 14 users hold both badges and would otherwise see six tiles of the same three faces. So the pair PR does not handle this; it is where the disagreement comes from.
This is a real break in both directions, and neither repo's CI catches it. At runtime, a SUPPORT_SURVIVOR holder opening the picker sees tiles keyed badge.SUPPORT_SURVIVOR.beetle|shell|peek; POST /update-user validates against avatarPoolOf(heldBadges), which contains only badge.SUPPORT_SURVIVOR.bandage|headset|beer, so every tap on that group 400s with 'Avatar not unlocked' and the picker snaps back with the generic save-failed toast. Conversely the three keys the API does bless resolve to no art here — avatarSrc returns null and UserAvatar silently drops to the first-letter fallback. On the API side, validateUiAssets would flag the three missing files, but it only runs when a peanut-ui root is found, and per the comment above resolveUiRoot API CI has no peanut-ui checkout, so pnpm badge:check passes there. On this side, avatar.utils.test.ts only asserts that every slug in the vendored manifest has a file in public/ — true, because both were hand-written together — so it cannot see the drift either.
The file header of avatar.utils.ts states this manifest is regenerated by pnpm badge:check --write-manifest in peanut-api-ts and 'never hand-edited', i.e. it claims to be a verbatim copy of the API artifact; that claim is what makes this blocking rather than a normal open-pair mismatch.
Fix: take whichever set the API PR lands with (currently bandage/headset/beer), copy the API's docs/badge-assets.json into src/types/badge-assets.json verbatim, and rename or redraw the three SVGs under public/avatars/badge/SUPPORT_SURVIVOR/ to match. If the intent was actually to reuse Bug Whisperer's faces, the change belongs in the API's BADGE_CATALOG so the generated manifest is the one that moves.
Checked clean
- Verified the supplied detached worktree head, trusted author, dev base ref, exact base SHA, and PR merge base.
- Rechecked P1: every tap still starts an independent update and refresh, with no serialization or stale-response guard; the tests cover only one settled save at a time.
- Rechecked P2: UserAvatar now uses static palette token classes instead of an inline style; the local design-system ratchet and exact-head ds-lint check both pass.
- Reviewed avatar key whitelisting, badge-derived unlock pools, optimistic rollback, user refresh propagation, profile deep-link state, badge-earned handoff, and fallback rendering.
- Inspected all 71 local avatar SVGs for active content and external references; none contain scripts, event handlers, foreign objects, images, uses, styles, or hrefs.
- Checked locale key parity for the full app bundles, manifest-to-file coverage, diff whitespace, focused tests, and the cross-repository API boundary available from the sibling policy checkout.
- Exact-head CI is complete: ci-success, unit, typecheck, eslint, format, analyze, ds-lint, ds-shots, human-authors, bot-approval, preview deployment, and review checks all succeeded.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 5b27877336f4 · Context: repo, api · Took 21m (queued 4m)
Guard manifest lookups against prototype names; pending-override save with one refetch per burst; roving focus from the pick; one source for the picker URL param; human tile labels and an accessible picture branch; explicit toast tap targets; shared avatar size map; fixtureHref for query-carrying fixture routes; DS shadow token on the selected tile. The manifest now lists every catalog badge, [] where art is pending.
Twenty basics at once was a wall. The picker now offers five — the current pick if it is a basic plus random fill — and the dice rerolls that row and never the pick, Split's semantics: the dice changes what is offered, not who you are. Dice, not die, everywhere users read it.
Two taps whose responses land out of order: the refetch fires once when the last request lands, so the newest write is what the tile shows, and a failed older request can no longer roll back a newer success. Deferred-promise tests cover both orders. Also prettier on the copied manifest.
It is the legacy code of Bug Whisperer, so it unlocks nothing; the manifest lists it as [] like every badge still waiting for art.
…e (TASK-22142) In demo and fixture mode a pick made through the picker vanished on the refetch that follows it, so the tile snapped back. The demo user now holds avatarKey (string sets, null clears, absent leaves it), and users/me returns it; fixture overrides still merge on top.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: the P1/P3 save race and P4 fallback mismatch remain, and demo-mode refetch discards saved picks. P2 is fixed; P5 is not reachable behind the protected-route hydration gate.
Findings
-
MAJOR · src/components/Avatar/AvatarPicker.tsx:54 · Serialize avatar writes so the last tap wins
The in-flight counter only postpones the refetch; it does not order the POSTs. If A is tapped, then B, and B commits before slower A, the server ends on A and the final fetch faithfully rolls the UI back to the first tap. The deferred test hard-codes B into mockFetchUser instead of modeling server state. Queue or coalesce writes so B is sent after A settles (or version them server-side), and test the backing state under both completion orders. -
MINOR · src/features/home/views/HomeTopNav.tsx:38 · Keep the fallback initial tied to the username
HomeTopNav receives avatarName, which useHomeFlow still replaces with fullName when showFullName is true and also uses fullName when the username is absent. With username alice, fullName Bea, and no avatar key, home renders B while the self profile renders A, contrary to this PR's username-initial fallback. Pass the username specifically to UserAvatar and keep display-name preferences out of the avatar seed. -
MAJOR · src/utils/demo-api.ts:388 · Read the mutable demo avatar during auth refresh
This mutable /users/me response is bypassed by useUserQuery, whose demo-mode branch dispatches the static DEMO_USER directly. After a demo user picks an avatar, POST /update-user sets demoAvatarKey, but AvatarPicker immediately awaits fetchUser(); that refresh restores static DEMO_USER and clearing pending snaps back to the initial. Route demo-mode fetchUser through the demo /users/me handler (or share the same mutable profile state) and cover save followed by the real auth refresh. -
MINOR · src/i18n/app/messages/es-AR.json:5 · [moonshotai/kimi-k3] es-AR bundles drop userAvatarAlt, which the new UserAvatar still uses
UserAvatar (added in this PR) calls useTranslations('common') and sets the picked avatar's role="img" label via t('userAvatarAlt', { username: name }); en.json keeps common.userAvatarAlt unchanged. This PR deletes the key from es-AR.json (and es-AR.marketing.json) — apparently as DotFaceAvatar cleanup, but DotFaceAvatar was replaced, not the key. For es-AR users the aria-label now falls back to the English string, or, where a locale is not merged over en, next-intl's missing-message fallback renders the raw key path as the accessible name. Fix: restore "userAvatarAlt": "Avatar de {username}" in both es-AR files, or point UserAvatar at a key that exists in every locale. -
MAJOR · src/components/Avatar/AvatarPicker.tsx:58 · [moonshotai/kimi-k3] A rejected avatar save leaks the in-flight counter and wedges the picker
In save(), inFlight.current += 1 is followed byconst { error } = await updateUserById(...)with no try/finally, and the call site is fire-and-forget (void save(key)). If the action rejects — offline/network failure or any thrown transport error, as opposed to a 400 returned as { error } — the decrement, the error toast, fetchUser() and setPending(undefined) are all skipped and the rejection is unhandled. The counter never returns to 0, so every subsequent tap still writes to the API but never triggers the reconciling fetchUser() or clears pending: the failed pick stays checked with no error shown, and the home slot / profile header keep rendering the stale avatar until a full page reload. Fix: wrap the call in try/catch (toast on catch) and doinFlight.current -= 1plus theinFlight.current === 0reconciliation in a finally block.
Checked clean
- Exact detached head, trusted author, dev base SHA, and merge base matched the supplied review target.
- SUPPORT_SURVIVOR now has an empty manifest entry and no asset directory; all 20 basic and 48 declared badge SVGs are present, with no executable or external SVG references.
- Protected /profile rendering waits for the user query before mounting Profile, so the prior P5 initial-hydration scenario is not reachable on the shipped route.
- Avatar path construction rejects unknown and prototype-derived keys; badge-toast routing and self-versus-public profile avatar behavior were reviewed.
- All checks at the exact head completed green, including unit, typecheck, eslint, format, ds-lint, ds-shots, analyze, preview, and provenance gates.
Second opinion by moonshotai/kimi-k3: 2 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 174df8fb9503 · Context: repo, peanut-api-ts sibling · Took 18m (queued 2m)
…-22142) Art direction ruling: the geometric vectors are replaced by the illustration pipeline's sticker art, so an avatar looks like the rest of Peanut's badge art instead of a second, flatter system next to it. WebP, not SVG, because the art is drawn, not diagrammed. These render at 32px on home and 88px on the profile header; a 176px WebP is exact at both and stays one file. Vectorizing raster art of this kind costs far more bytes than it saves. `avatarSrc` and the tests build `.webp` paths now; nothing else changes, since the manifest names slugs and never file types. Every slug and directory in the manifest is unchanged, so this is an art-only diff in one repo: no peanut-api-ts edit, no manifest regen, no contract to sequence. Slug words and the art they now name have drifted apart (basic.apple draws a cherry) — that is an API-side rename, tracked separately. Day 0 gets art too: a user who has not picked wears their initial as a graffiti sticker rather than a letter in a circle. `letterAvatarSrc` is deliberately not a manifest entry — a letter is never a pick, only what an unpicked user looks like. A name that does not start with a-z still falls through to the first-letter avatar, and AvatarWithBadge is untouched, so counterparty rows keep their initials. UserHeader linked to /profile but drew a bare initial, so a user's own avatar changed shape between /home and that header. It renders UserAvatar now.
|
@/tmp/claude-1000/-workspaces-sandbox/d309fdf1-950a-4136-8a45-848ea17601d6/scratchpad/comment.md |
There was a problem hiding this comment.
Chip review — changes requested
Request changes: rapid avatar taps can still persist an earlier choice, and the home fallback can still disagree with the self-profile fallback by deriving its initial from the full name. The manifest, demo refresh, and locale concerns are resolved at this head.
Findings
-
MAJOR · src/components/Avatar/AvatarPicker.tsx:54 · Serialize avatar writes so the last tap wins
Two rapid taps still start two independent POSTs. If A is tapped first, B second, B commits first, and slower A commits last, the database ends on A; waiting for the in-flight count to reach zero only makes the final refetch reveal A. The deferred tests force the refetch mock to return B instead of applying each write in completion order. Queue or coalesce saves so B cannot be sent until A finishes, then refetch once after the queue drains. -
MINOR · src/features/home/views/HomeTopNav.tsx:38 · Keep the fallback initial tied to the username
For usernamealice, full nameZelda Example,showFullName=true, and no avatar key,useHomeFlowpasses the full name asavatarName, so this line renders the Z sticker on home while the self profile passes the username and renders A. Pass the username toUserAvatarhere, retaining the generic fallback only when no username exists. -
BLOCKING · src/components/Avatar/avatar.utils.ts:52 · [claude-opus] Avatar art ships as .webp, but the API half checks and documents .svg
peanut-api-ts#1498 — the other half of TASK-22142 — builds the cross-repo asset paths itself, with the extension baked in:scripts/check-badge-catalog.ts, validateUiAssets():
for (const slug of badge.avatars) check(code,/avatars/badge/${code}/${slug}.svg)
for (const slug of BASIC_AVATAR_SLUGS) check('basic',/avatars/basic/${slug}.svg)
and docs/BADGES.md in the same PR tells every future badge author to "Add 1–3 profile avatars next to it under public/avatars/badge/<CODE>/<slug>.svg".
This PR ships 0 .svg and 94 .webp under public/avatars (48 badge + 20 basic + 26 letter), and src/components/Avatar/avatar.utils.ts:52,54,85 builds .webp paths. The manifest itself (src/types/badge-assets.json) matches the API's byte-for-byte, so the slugs agree — only the file type does not, and the API is the side that decides the file type when it constructs the path.
Where it breaks, concretely:
pnpm badge:checkin peanut-api-ts resolves a UI root automatically from a siblingpeanut-uicheckout (scripts/check-badge-catalog.ts:78-81), and BADGES.md:305 tells devs to run--ui-root ../peanut-ui --require-ui. With both PRs on disk that run throwsMissing UI badge assetslisting all 68 badge+basic paths. API CI runspnpm badge:checkwith no UI root (.github/workflows/tests.yaml:89), so CI stays green and this only bites on a developer machine and on the very check that is supposed to be the cross-repo guard — it validates nothing real today.- Durably: a badge author who follows BADGES.md adds
<slug>.svg, badge:check passes, and the picker then renders a broken tile, because avatarSrc only ever asks for<slug>.webp.
The head commit says "Every slug and directory in the manifest is unchanged, so this is an art-only diff in one repo: no peanut-api-ts edit, no manifest regen, no contract to sequence" and "nothing else changes, since the manifest names slugs and never file types". The manifest indeed names only slugs — but the API's validator and its docs supply the extension, so the WebP ruling is not confined to this repo.
Fix: this needs a paired edit in peanut-api-ts#1498 before that PR merges — change the two .svg literals in validateUiAssets to .webp and the public/avatars/badge/<CODE>/<slug>.svg line in docs/BADGES.md to .webp. If the extension is meant to be the UI's to choose, put it in the manifest (e.g. an avatars.ext field) instead of hardcoding it on the API side. I can only see peanut-api-ts#1498 as posted, not its current head, so if that PR has already been moved to .webp this is already handled.
Checked clean
- Verified the detached worktree HEAD, exact merge base, trusted PR author, base ref, and base SHA without fetching issue or review comments.
- Re-evaluated P1 and P3 as the same rapid-tap race; the in-flight counter delays refetching but does not serialize backend writes, so P3 remains actionable.
- Re-evaluated P2 against paired API commit b165c6c8: the UI manifest matches the generated API manifest semantically, SUPPORT_SURVIVOR is empty in both, and exact-head unit and asset checks pass.
- Re-evaluated P4 and confirmed the home fallback still receives the full-name-derived avatarName while ProfileHeader uses the username.
- Re-evaluated P5: demo /users/me now reads the mutable demoAvatarKey and the exact-head unit test covers set, preserve-on-omission, and clear.
- Re-evaluated P6: es-AR is a delta catalog merged over es-419, so removing the duplicate userAvatarAlt override does not drop the resolved key.
- Re-evaluated P7: updateUserById catches fetch and JSON failures and resolves them as { error }, so its production contract does not reject and cannot leak the counter on that path.
- Reviewed avatar key validation and path construction, badge unlock rendering, picker failure and keyboard paths, profile/home integration, badge-earned deep linking, demo fixtures, and removed DotFaceAvatar references.
- Exact-head CI completed successfully for unit, typecheck, ESLint, format, design-system lint and screenshots, analysis, preview deployment, provenance, and aggregate success checks.
- Focused tests could not be rerun locally because the detached worktree has no node_modules; exact-head CI supplied the test evidence and no worktree files were changed.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 12442e8f3373 · Context: repo, other_repository · Took 21m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Three findings remain: concurrent taps can persist the older avatar, the home fallback can use the full-name initial, and the paired API asset validator still requires SVG while this UI ships WebP. CI is green at the exact head.
Findings
-
MAJOR · src/components/Avatar/AvatarPicker.tsx:54 · Concurrent saves can commit the older tap last
Tap avatar A, then B while A is pending. Both update-user calls run concurrently; if B completes first and A completes last, A is the final database write. The single refetch then restores A, so the first tap wins. The new test hard-codes the mock server to B and cannot catch this. Serialize or coalesce writes so only one is active and any newer pending key is sent after it. -
MINOR · src/features/home/views/HomeTopNav.tsx:38 · Fallback avatar still follows the full name
For usernamebob, full nameAlice Example, and showFullName=true, useHomeFlow supplies the full name as avatarName, so this renders the A sticker and hashes its palette from the full name. The fallback is meant to stay keyed to the username; pass username to UserAvatar independently of the displayed name. -
MAJOR · src/components/Avatar/avatar.utils.ts:52 · Cross-repo asset check still expects SVG
The paired API's documentedpnpm badge:check --ui-root ... --require-uibuilds.svgpaths for all 68 profile avatars. This PR's resolver and every added avatar file use.webp, so that check reports every avatar missing even though the UI's own test passes. Align the paired checker/docs and the UI asset format before rollout, or ship the SVG files the contract names. -
MAJOR · src/components/Avatar/avatar.utils.ts:52 · [claude-opus] Avatar art is .webp here, .svg in the API half's asset check
This PR ships every avatar as WebP (public/avatars/basic/.webp, public/avatars/badge//.webp, public/avatars/letter/*.webp) and avatarSrc/letterAvatarSrc build.webppaths (src/components/Avatar/avatar.utils.ts:52, :54, :85). The manifest carries slugs only, so the extension is the unwritten half of the contract — and the other side spells it.svg.
Evidence, from the matching open PR peanut-api-ts#1498 (not in the pinned checkout, so this is the pair's other half): scripts/check-badge-catalog.ts gains for (const slug of badge.avatars) check(code, /avatars/badge/${code}/${slug}.svg) and for (const slug of BASIC_AVATAR_SLUGS) check('basic', /avatars/basic/${slug}.svg), and docs/BADGES.md documents public/avatars/badge/<CODE>/<slug>.svg as the required art. validateUiAssets already runs against any sibling UI checkout — resolveUiRoot auto-detects ../peanut-ui even with no --ui-root flag (scripts/check-badge-catalog.ts in the pinned checkout, resolveUiRoot). So once both halves land, pnpm badge:check on any machine with both repos checked out throws Missing UI badge assets, listing all 71 declared avatar slugs, and the docs tell the next badge author to commit SVGs that this repo will never render.
Fix on one side, not both: either change the two .svg literals and the BADGES.md path in peanut-api-ts#1498 to .webp, or re-export the art as SVG here. WebP looks deliberate (the files are 6–17KB raster stickers), so the API half is the one to change. Note this is a pairing, not a merge blocker for this repo alone: nothing in peanut-ui breaks at runtime, and the UI-side test in src/components/Avatar/tests/avatar.utils.test.ts already asserts the .webp files exist.
Checked clean
- Detached HEAD, exact base, merge base, trusted author, and PR target all match the supplied values.
- CI at the exact head is green, including unit, typecheck, eslint, format, native-export, ds-shots, and ci-success.
- P2 is fixed: the UI and paired API avatar manifests agree semantically, SUPPORT_SURVIVOR has no set, and all 68 declared avatar assets exist as WebP.
- P5 is fixed: demo update-user mutates demoAvatarKey and users/me reads that mutable value.
- P6 does not apply: es-AR is a delta catalog merged over es-419, which contains the new avatar strings.
- P7 does not apply: updateUserById catches transport and parsing failures and returns an error result, so the counter decrement path still executes.
- Unknown and traversal-shaped avatar keys cannot form image paths because avatarSrc accepts only manifest-listed slugs.
- Profile/self surfaces, badge-toast handoff, picker keyboard behavior, fixtures, and demo-state wiring were reviewed.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 60f9c09026a1 · Context: repo, sibling-repo, ci · Took 12m
…demo fetch reads the demo profile (Chip, #2929) Saves now go one at a time, always the latest tap next, with one refetch in a finally, so the server can never commit an older key last and a thrown save cannot wedge the picker. The home chip's letter and palette seed from the username like the profile header, not the display name. In demo mode fetchUser reads the demo /users/me handler, so a pick survives the refetch that follows it.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
One last-tap-wins race remains in the serialized avatar save drain; the other supplied prior findings are fixed at this head.
Findings
- MAJOR · src/components/UserHeader/index.tsx:21 · [moonshotai/kimi-k3] UserHeader calls useAuth() with no visible import
src/components/UserHeader/index.tsx line ~21 addsconst { user: authenticatedUser } = useAuth(), but the import hunk (lines 1–6) only swaps AvatarWithBadge for UserAvatar — noimport { useAuth } from '@/context/authContext'is added anywhere in the diff. Unless the file already imported useAuth (nothing in the old hunk suggests it did — the component previously took onlyusernamefrom props), this is a TS/compile error that breaks the build, and there is no UserHeader test in the diff to catch it. Fix: add the useAuth import (and cover UserHeader with a render test).
Answered by a human, not raised again
These are still present as far as this review can tell. You resolved the thread or deferred the fix, so Chip files them instead of repeating them.
- MAJOR · src/components/Avatar/AvatarPicker.tsx:54 · Drain taps queued during the final refetch — task
Checked clean
- Confirmed the detached worktree head, merge base, trusted author, and dev base exactly match the supplied values.
- Rechecked the supplied prior findings: username fallback identity, demo profile refresh, Support Survivor's empty set, WebP paths, and es-AR inheritance are fixed at this head.
- Checked avatar-key parsing and public-path construction against malformed and unlisted keys; paths are manifest-whitelisted rather than interpolated from arbitrary input.
- Checked manifest/art parity: all 68 selectable manifest assets exist with no extras, and all 94 avatar files including letter fallbacks have RIFF WebP signatures.
- Reviewed profile, home, badge-toast, fixture, and locale integration for regressions.
- Exact-head CI completed successfully for unit, typecheck, eslint, format, native export, design-system lint, visual snapshots, analysis, and preview deployment.
- A focused local Jest rerun was unavailable because the detached worktree has no node_modules; the exact-head unit CI run is successful.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 7a9778d96524 · Context: repo, product, sibling-repo · Took 13m (queued 2m)
…e ring Art direction, 2026-09-03: a sticker carries its own colours and edge, so UserAvatar renders the picked or letter sticker at full size with nothing behind it. The palette-triple hash and its classes go with the circle.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
One major concurrency defect remains: a selection made during the final profile refetch is dropped and never sent.
Findings
-
MAJOR · src/components/Avatar/AvatarPicker.tsx:64 · Drain taps queued during the final refetch — also flagged by moonshotai/kimi-k3
After the last POST completes,drainexits the loop and awaitsfetchUser(). If the user taps another avatar during that await,savestores the new key inwanted.currentbut seesdraining.current === true, so it does not start another drain. When the refetch resolves,finallyclears the optimistic pick without consuming that key: the server keeps the previous avatar and the UI snaps back. Before clearingpending, restart or continue the drain wheneverwanted.currentwas populated during the refetch, and cover that timing with a deferred-refetch test. -
MAJOR · src/components/Avatar/AvatarPicker.tsx:64 · [claude-opus] Tap during the closing refetch is dropped, and no test covers it
The picker mutates persisted user state (users.avatar_key via POST /update-user), so the hard rule applies to every path throughdrain. The burst path is tested well —sends one save at a time…anda rejected first save…both pin the serialized ordering — but the window after the loop drains is not.
In src/components/Avatar/AvatarPicker.tsx:54-67 the while (wanted.current !== undefined) loop exits, then await fetchUser() runs with draining.current still true. A tap during that await goes through save, which sets wanted.current = key and then skips starting a drain because draining.current is true (line 75). The loop it was queued for has already ended, so the POST is never sent. The finally then does setPending(undefined) (line 67), so the tile visibly snaps back to the previously committed key and the user's last tap is silently lost — no toast, no retry.
The untested case, exactly: tap A, settle A's POST, tap B while fetchUser() is still in flight, then resolve fetchUser(). Expected: a POST for B is sent and B ends up committed. Actual: mockUpdateUserById is called once, with A.
Fix: drain again after the refetch instead of dropping what queued during it — e.g. wrap the loop-plus-refetch in do { …loop…; await fetchUser() } while (wanted.current !== undefined), and only clear draining/pending once wanted.current is undefined. Add the four-step test above alongside the existing serialization tests.
Checked clean
- Confirmed the detached worktree HEAD and merge base exactly match the supplied head and base SHAs, then reviewed the complete base-to-head diff.
- Checked avatar key parsing, badge unlock filtering, URL-state entry points, profile/home rendering, keyboard behavior, failure handling, and demo persistence; no additional correctness or security defect survived adversarial review.
- Verified the UI manifest declares 20 basic and 48 badge avatars, SUPPORT_SURVIVOR declares none and has no asset directory, and every declared avatar plus all 26 letter fallbacks is present.
- Verified es-AR resolves missing avatar strings through its es-419 base catalog.
- Exact-head CI is green for typecheck, lint, formatting, unit tests, native export, design-system checks, and screenshots; the older failed aggregate belonged to a superseded run whose required jobs were canceled.
- git diff --check passes.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: e7fbf4919679 · Context: repo, sibling-api, ci · Took 8m (queued 3m)
Chip: after the last POST the drain awaited fetchUser with draining still set, so a tap in that window queued on wanted and was cleared by the finally without ever being sent. The loop now repeats until nothing is queued after the refetch; test covers tap A, settle A, tap B mid-refetch.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable defects remain at this head; the closing-refetch race and all earlier concerns are fixed or refuted.
Findings
- MINOR · src/components/UserHeader/index.tsx:35 · [moonshotai/kimi-k3] UserHeader renders the authenticated user's avatar under whatever username prop it is given
UserHeader takes ausernameprop and displays it, but unconditionally passesauthenticatedUser?.user.avatarKeyto UserAvatar. If this component is ever rendered for anyone other than the logged-in user (e.g. reused on a counterparty or search-result header), it shows the viewer's picked avatar next to someone else's name, misattributing identity. Fix: compareusernametoauthenticatedUser?.user.username(case-insensitively, as ProfileHeader does) and only pass avatarKey when they match, otherwise render the plain initial avatar.
Checked clean
- Confirmed the detached worktree HEAD, exact base SHA, merge base, trusted author, and dev target.
- Reviewed the complete 131-file diff across avatar selection, profile/home integration, badge toast routing, demo state, fixtures, translations, tests, and assets.
- Verified serialized last-tap-wins saves, queued taps during the closing refetch, failed-save recovery, and the focused concurrency tests.
- Verified manifest/path guards, prototype and traversal rejection, 68 unique safe selectable keys, and Support Survivor's empty avatar set.
- Decoded all 94 added WebP assets: every file is a valid 176x176 RGBA WebP.
- Verified username-based fallback identity, es-AR inheritance through es-419, mutable demo avatar refetches, and picker deep-link query composition.
- Checked exact-head CI: unit, typecheck, eslint, format, design-system, analysis, provenance, and ownership checks succeeded.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 427292f585c3 · Context: repo · Took 12m (queued 11m)
|
Folded in the approved profile-header change (9e6f579): on your own profile the avatar, the username line and the share pill are now one 72px pill — avatar segment opens the picker, the rest shares, two hit areas inside one border rather than a nested button. Other people's profiles keep the stacked header. Spec: https://claude.ai/code/artifact/45ec1b09-b7c5-4d7c-a0f1-d6bfd8162d00 Also returned the verified check to |
Conflicts in useHomeFlow: dev (#2929, TASK-22142) removed avatarName and seeds the top-nav chip from the username directly, which supersedes this branch's avatar-seeding fix. Took dev's version of the hook and its test.
Summary
Badge-linked profile avatars (v2). The generated dot-face avatar was not launch-ready and derived from name strings. Now the home slot and the self profile show the user's picked avatar — one of twenty basics everyone gets, or one of the one-to-three avatars each badge unlocks — with a privacy-safe fallback: exactly one username initial on the username's color, never the full name or verification data.
UserAvatar(picked key → svg on its palette, otherwise the fix(native): native release bug fixes — 2026-09-02 (TASK-22125 duplicate notification artwork, TASK-22146 About ToS title locale, TASK-22209 duplicate welcome push, card details survive app switch + expiry copy) #2924 first-letter avatar) inHomeTopNavand the selfProfileHeader, which becomes a button that opens the picker.AvatarPickeris aGlobal/Drawer: "From your badges" (only badges the user holds) above "Basics", a tap saves at once, the die randomizes across the whole pool (free, forever), "Use my initial instead" clears. Open state is nuqs URL state, so/profile?avatarPicker=truedeep-links.BadgeEarnToastgains a second line, "N new avatars unlocked", with a "Choose avatar" control that routes into the picker. The badge tap keeps its detail view.badge-assets.json(regenerated by the API'sbadge:check) now carries the avatar slugs; the UI mirrors it into paths and asserts every file exists. Art:public/avatars/basic/<slug>.svg,public/avatars/badge/<CODE>/<slug>.svg.Task
TASK-22142 — https://app.notion.com/p/peanutprotocol/Build-badge-linked-avatar-system-v2-3cf83811757981efa8afecf6667d2662
Risks / breaking changes
feat/22142-badge-avatars:users.avatar_key,POST /update-user { avatarKey },user.avatarKeyon/users/me) deployed first. Until then the slot shows the initial fallback and a pick fails with the friendly error toast; nothing crashes.AvatarWithBadge firstLetterOnly);UserAvataronly adds the picked-key branch and otherwise renders that same fallback.updateUserByIdalready takes a Record,check:apiuntouched.QA
/home?__fixture=home-avatar(basic frog in the slot),/profile?avatarPicker=true&__fixture=avatar-picker(picker open, Bug Whisperer avatars unlocked, beetle selected). Fixture routes may now carry their own query.Design
Interactive design and demo of this system (rendered from this branch's art and manifest): https://claude.ai/code/artifact/774884e3-d479-4ece-9423-63d82600e333
Screenshots
Sticker art set (12442e8), rendered plain on the page at 88 / 32 / 24 px. Assets live on the
pr-assets-2929branch, deleted after merge.?__fixture=avatar-picker,?__fixture=home-avatar, 375×667)Design notes / accepted trade-offs
avatarPaletteClasshelper went with the circle.