fix: avatar upload display and oversized crop on HiDPI screens

[nickmeinhold]

Summary

Two related avatar upload bugs:

1. getAvatarUrl() returns empty string for S3 keys — uploaded avatars never display because the function only handles full URLs, not S3 keys. This implements the URL construction logic that the existing tests in helpers.test.ts already describe, supporting both path-style (MinIO/LocalStack) and virtual-hosted (Tigris/AWS S3) URLs.

2. Avatar crop produces oversized blobs on HiDPI screens — the crop canvas was scaled by devicePixelRatio (2x on Retina = 4x pixel count), and toBlob() was called without a quality parameter. A 782KB source image would produce a >2MB blob, exceeding S3_AVATAR_UPLOAD_LIMIT. Now caps output at 512x512px (more than enough for avatars displayed at 64x64) and uses quality=0.85.

Changes

• apps/web/src/utils/helpers.ts — implement S3 key → URL construction in getAvatarUrl(), using NEXT_PUBLIC_STORAGE_URL, NEXT_PUBLIC_AVATAR_BUCKET_NAME, and optionally NEXT_PUBLIC_USE_VIRTUAL_HOSTED_URLS + NEXT_PUBLIC_STORAGE_DOMAIN
• apps/web/src/views/settings/components/Avatar.tsx — replace devicePixelRatio scaling with 512x512 cap, add quality=0.85 to canvas.toBlob()

Test plan

• All 8 existing getAvatarUrl tests pass (they were previously failing against the stub implementation)
• Path-style URL construction verified with self-hosted MinIO (storage.imagineering.cc)
• Avatar upload tested on Retina MacBook — blob stays well under 2MB limit

Closes #440, closes #441

[nickmeinhold]

Automated Code Review — Cage Match Edition

Two AI reviewers independently analyzed this PR:

---

MaxwellMergeSlam's Review 🤼

Verdict: APPROVE

Summary: Clean, focused two-bug fix that solves real user-facing issues — this PR does exactly what it says on the tin and doesn't try to boil the ocean.

John McClane: "Come out to the coast, we'll get together, have a few laughs..." — that's what devicePixelRatio said to your upload limit before this fix body-slammed it through the announcer's table.

Findings:

• helpers.ts:56-69 — URL construction logic is clean. Virtual-hosted vs path-style branching follows the same pattern already established in the backend S3 code. No path traversal risk since the key comes from the upload flow, not user input.
• helpers.ts:1 — next-runtime-env import added at top level. This is the correct way to read NEXT_PUBLIC_* vars at runtime in Next.js (as opposed to process.env which bakes values at build time).
• Avatar.tsx:116-118 — The Math.min(maxSize / cropWpx, maxSize / cropHpx, 1) calculation is elegant. The , 1 clamp means small images don't get upscaled. Nice.
• Avatar.tsx:141 — quality=0.85 is a sensible default for JPEG — visually indistinguishable from 1.0 at avatar sizes, massive size reduction.
• Avatar.tsx:136 — selectedFile?.type ?? "image/jpeg" fallback is fine for avatars.

The Good:

• Minimal, surgical diff — 2 files, +18/-10 lines. No scope creep.
• Test coverage already exists in helpers.test.ts with 8 tests covering null, empty, absolute URLs, path-style, and virtual-hosted variants. The tests were written first and the implementation makes them pass. ATDD done right.
• The HiDPI fix correctly identifies the root cause (canvas scaling by devicePixelRatio squaring pixel count) rather than just bumping the upload limit.
• Good comments explaining why 512 was chosen (avatars display at 64x64).

The Concerns:

• Minor: quality=0.85 only applies to JPEG and WebP. For PNG uploads, toBlob ignores the quality param — not a bug (PNGs are lossless), but worth noting. Since avatars are typically photos, JPEG is the common case and this is fine.
• Nit: If both storageUrl and storageDomain are unset, getAvatarUrl() returns "" for a valid S3 key — silently fails. This matches the pre-existing behavior though, so it's not a regression.

Rocky Balboa: "It ain't about how hard you hit. It's about how hard you can get hit and keep moving forward." This PR took a Retina screen to the face and kept going. APPROVE.

---

KelvinBitBrawler's Review

Verdict: APPROVE

Summary: This glacial patch finally thaws out two critical avatar upload defects, bringing the system back from the brink of a digital deep freeze with chilling efficiency.

Findings:

• apps/web/src/views/settings/components/Avatar.tsx:110-112: The previous devicePixelRatio scaling was a quantum leap backward for file sizes, an absolute thermodynamic nightmare, producing blobs that would make a black hole jealous.
• apps/web/src/utils/helpers.ts:50: The original getAvatarUrl was a black hole for S3 keys, absorbing them into nothingness, leaving us with a cosmic void where avatars should be.

The Good:

• apps/web/src/utils/helpers.ts:
  • Character: "All these moments will be lost in time, like tears in rain." But these S3 URL constructions, they will not be lost. The logic for generating virtual-hosted and path-style S3 URLs is precisely what was missing. It's a cold, hard fact that this was needed.
  • The use of next-runtime-env for environment variables is standard procedure. Keeps things orderly, avoids unexpected thermal fluctuations in different deployment environments. This is a solid, calculating move.
• apps/web/src/views/settings/components/Avatar.tsx:
  • The maxSize = 512 cap is a brilliant move to prevent runaway data growth. Avatars are meant to be small, not cosmic. This brings the pixel count back to a sensible, low-entropy state, preventing a catastrophic data meltdown.
  • Adding quality=0.85 to canvas.toBlob() is the final piece of the puzzle. This thermal compression ensures we're not sending gigabytes to display a thumbnail. It's efficient, it's brutal, it's correct.
  • Character: "I can't allow that, Dave." The previous devicePixelRatio scaling was an anomaly that threatened the very fabric of our data budget. Removing it, and the setTransform nonsense, was absolutely essential. A bold, necessary incision.

The Concerns:

• apps/web/src/views/settings/components/Avatar.tsx:115: The maxSize constant, currently defined within onCropComplete, is a slight thermal inefficiency. It's a trivial re-declaration on every crop. Hoist it. It's not going to crash the system, but it's a minor crack in the pristine ice of optimized code.

---

Consensus

Both reviewers APPROVE. The fix is minimal, well-tested, and correctly addresses both root causes.

[nickmeinhold]

Hey @hjball 👋 friendly ping on this one — it's been sitting for a couple of weeks. Small fix (2 files, ~20 lines) for avatar upload display and HiDPI crop sizing. Happy to address any feedback if needed!

[nickmeinhold]

Separate topic — are you aware Anthropic offers free Claude credits to open source maintainers? Not sure if you're already using Claude or have a different AI setup, but figured it was worth mentioning in case it's useful for the project.

If you're not interested I'll happily take them off your hands 😄 (kidding... mostly)

[nickmeinhold]

Sorry for turning your PR into my personal support ticket 😅 but one more thing — the security advisory GHSA-qrx8-9hc6-jvqg still shows as published even though the fix landed in v0.5.5. Should be closeable from the Security tab since it's already patched. Would clear the warning banner from the repo.

I promise this is the last thing... probably.

[hjball]

Apologies for delay on PRs at the moment @nickmeinhold

Taking a look now!

[hjball]

Separate topic — are you aware Anthropic offers free Claude credits to open source maintainers? Not sure if you're already using Claude or have a different AI setup, but figured it was worth mentioning in case it's useful for the project.

If you're not interested I'll happily take them off your hands 😄 (kidding... mostly)

Yes, I've already applied but I think the threshold is 5k stars sadly (don't think it'll be too long before we get there)

[hjball]

Sorry for turning your PR into my personal support ticket 😅 but one more thing — the security advisory GHSA-qrx8-9hc6-jvqg still shows as published even though the fix landed in v0.5.5. Should be closeable from the Security tab since it's already patched. Would clear the warning banner from the repo.

I promise this is the last thing... probably.

I'm not too familiar with the security advisory process but I believe they can't be closed once published, right? I've updated the patched version in the advisory so hopefully it's a bit clearer now.

[hjball]

lgtm @nickmeinhold