fix(changelog): fallback contributor avatars

[botshen]

Summary

• keep GitHub contributor avatars as the primary changelog avatar source
• add DiceBear identicon fallback URLs using the previous avatar generation logic
• switch broken contributor images to the fallback avatar on load error

Tests

• npx vitest run src/pages/Changelog/utils.test.ts
• npm run build

[Jerry-Xin]

The PR is in scope for octo-admin and cleanly addresses changelog contributor avatar fallback behavior.

💬 Non-blocking

• 🔵 Suggestion: src/pages/Changelog/utils.ts:198 seeds DiceBear with the raw contributor name, so octocat and @octocat produce different fallback avatars even though githubAvatarUrl normalizes them to the same GitHub user. Consider reusing the normalized login for the fallback seed if consistent identity rendering matters.
• 🔵 Suggestion: src/pages/Changelog/index.tsx:408 uses event.currentTarget.onerror = null, which is a common DOM pattern, but React’s synthetic handler may still remain attached. A guard like “only swap if current src is not already the fallback” would make fallback failure handling more robust.
• 🔵 Suggestion: src/pages/Changelog/utils.test.ts:5 covers URL generation, but not the img error behavior. A small component test would protect the actual fallback swap.

✅ Highlights

• GitHub avatars remain the primary source.
• The fallback URL is generated deterministically and URL-encodes contributor names.
• The new type field is produced and consumed within the same changelog flow.

Verification note: I attempted npm run build, but this checkout lacks tsc (sh: tsc: command not found), so I could not independently verify the declared build command here.

[yujiawei]

Code Review — PR #79 (octo-admin)

Verdict: APPROVED

A small, well-scoped fix that adds a graceful fallback for changelog contributor avatars. GitHub profile images remain the primary source; if one fails to load, the <img> swaps to a deterministic DiceBear identicon. Unit test and production build both pass locally.

Verification

Check	Result	Evidence
Fallback wiring is correct	✅	src/pages/Changelog/index.tsx:405-411 — onError nulls currentTarget.onerror first (prevents infinite error loop), then swaps to c.fallbackAvatar. Correct order.
Fallback URL generation	✅	src/pages/Changelog/utils.ts:198-200 — deterministic per-name seed + indexed color from CONTRIBUTOR_COLORS, with index % length wrap so >8 contributors don't crash.
Type/contract update	✅	Contributor interface extended with fallbackAvatar (utils.ts:183-187); all producers (utils.ts:213-217) and consumers (index.tsx:406-410) updated consistently. No other usages found (grep of fallbackAvatar/c.avatar across src/).
Tests	✅	npx vitest run src/pages/Changelog/utils.test.ts → 1 passed. Test asserts both caster-Q and @octocat fallback URLs including URL-encoding of @ (%40octocat).
Build	✅	npm run build (tsc + vite) → built successfully, no type errors.

Findings

No P0/P1 issues. Two non-blocking nits below.

Nits (non-blocking)

1. Seed inconsistency for @-prefixed handles (utils.ts:194 vs utils.ts:199). The primary avatar strips the leading @ (name.replace(/^@+/, '') → octocat), but the fallback seed keeps it (encodeURIComponent('@octocat') → %40octocat). This is harmless — the fallback only needs a stable, unique seed — but if you ever want the fallback identicon to be "the same person" conceptually as the GitHub avatar, you'd strip @ in both. Purely cosmetic.

2. External dependency on api.dicebear.com. The fallback now depends on a third-party service. If both GitHub and DiceBear are unreachable, the image silently stays broken (acceptable — onerror is nulled so there's no loop). Per the PR description this restores the previously-used generation logic, so no behavior regression. Worth noting only as a future hardening candidate (e.g. a local/inline SVG fallback), not for this PR.

Summary

Clean, minimal, and correct. Tests and build green. Approving.

[lml2468]

✅ APPROVED with follow-up suggestions

Reviewed at HEAD b747eb3. Change directly addresses prior round's nits (onError fallback + @-prefix handling via encodeURIComponent). +14/-1, clean and focused.

🟡 P1 — src/pages/Changelog/index.tsx:408 loop guard is incomplete

onError={(event) => {
  event.currentTarget.onerror = null
  event.currentTarget.src = c.fallbackAvatar
}}

event.currentTarget.onerror = null clears the DOM property, but React uses delegated event handlers at the root — the synthetic onError prop is not cleared by this assignment. If the DiceBear fallback URL itself fails to load (CDN outage, ad-blocker, network policy), React will re-fire onError and re-assign the same fallbackAvatar src, creating a request loop.

Suggested guard:

onError={(event) => {
  if (event.currentTarget.src === c.fallbackAvatar) return
  event.currentTarget.src = c.fallbackAvatar
}}

Low real-world probability (DiceBear is reliable), so not blocking — but worth hardening since the cost is one line.

🟡 nit — test only verifies field shape, not fallback behavior

utils.test.ts now asserts fallbackAvatar is present in the parsed output, but there's no test that exercises the actual onError swap, nor edge cases like @-only names, empty entries, weird separators. The data-shape test catches regressions in URL generation but won't catch a regression where the <img onError> handler is removed or broken.

A small component test (RTL + dispatching an error event on the rendered <img>) would cover the behavior this PR is meant to harden.

🟡 nit — fallback color is index-positional, not name-stable

CONTRIBUTOR_COLORS[index % CONTRIBUTOR_COLORS.length]

A contributor's fallback color depends on their position in the list, so reordering or inserting names shifts everyone's color. Hashing the name (e.g. simple char-sum mod length) would make the color stable per contributor across releases. Pure cosmetic.

📋 Summary

No blockers. URL encoding is safe for malformed input, DiceBear fallback is a real improvement, tests assert the data contract. Merge OK — please consider the loop-guard hardening in a follow-up since it's a 1-line change.

RECOMMENDATION: APPROVE