Align notification avatars with IPFS gateway fallback#2229
Merged
Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 8 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds a client hook to manage image load retries across Arweave/IPFS gateways, integrates it into avatar and notification header components, expands IPFS URL rewriting to handle ipfs.io gateway URLs, updates image-scaling helpers, and adds tests and typecheck config changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant Hook as useGatewayImageLoadState
participant Image as Image Element
participant GatewayList as Fallback Gateways
Component->>Hook: useGatewayImageLoadState(src)
Hook->>GatewayList: getArweaveGatewayFallbackUrls(src)
GatewayList-->>Hook: [gateway1, gateway2, ...]
Hook->>Hook: Initialize: mode="optimized", candidateIndex=0
Hook-->>Component: activeSrc=gateway1, unoptimized=false
Component->>Image: Render <Image src={activeSrc} unoptimized={false} />
Image->>Image: Load attempt from gateway1
Image--XImage: Load fails
Image->>Component: onError triggered
Component->>Hook: handleError()
Hook->>Hook: Switch to mode="unoptimized"
Hook-->>Component: activeSrc=gateway1, unoptimized=true
Component->>Image: Re-render <Image src={gateway1} unoptimized={true} />
Image->>Image: Load attempt (unoptimized)
Image--XImage: Load fails again
Image->>Component: onError triggered
Component->>Hook: handleError()
Hook->>Hook: Increment candidateIndex, mode="optimized"
Hook-->>Component: activeSrc=gateway2, unoptimized=false
Component->>Image: Re-render <Image src={gateway2} unoptimized={false} />
Image->>Image: Load attempt from gateway2
Image-->>Image: Success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/brain/notifications/subcomponents/NotificationHeader.tsx`:
- Around line 25-33: activeSrc returned from useGatewayImageLoadState() is
already the resolved concrete URL; wrapping it in getScaledImageUri() causes
resolveIpfsUrlSync() to run again and undo gateway fallback behavior. Fix: when
rendering the Image in NotificationHeader (and likewise in OverlappingAvatars),
detect if activeSrc is already resolved and avoid calling getScaledImageUri on
it — instead apply scaling directly to the resolved URL or use a helper that
only appends sizing params without re-running resolveIpfsUrlSync; update the
Image src usage to pass the already-resolved activeSrc (or a non-resolving
scaling helper) so gateway fallback attempts are preserved.
In `@components/ipfs/IPFSContext.tsx`:
- Around line 90-106: The rewrite currently only replaces
parsedUrl.hostname/host using getConfiguredIpfsGatewayHost(), which loses any
configured port or base path; instead, parse the full configured gateway
endpoint and reconstruct the rewritten URL by adopting its protocol, hostname,
port and base pathname. In practice: call new URL(configuredGatewayEndpoint)
(from the value returned by getConfiguredIpfsGatewayHost() or adjust to fetch
the full endpoint), then set parsedUrl.protocol, parsedUrl.hostname,
parsedUrl.port and set parsedUrl.pathname = joinPaths(configuredBase.pathname,
parsedUrl.pathname) (ensuring single slashes) and update host accordingly so
functions/variables in this block (parsedUrl, configuredHost /
configuredGatewayEndpoint, parsedUrl.pathname) preserve port and non-root base
paths when rewriting ipfs.io URLs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52af748d-6924-41eb-8724-bd48c902f8f7
📒 Files selected for processing (8)
__tests__/components/brain/notifications/subcomponents/NotificationHeader.test.tsx__tests__/components/common/OverlappingAvatars.test.tsx__tests__/components/ipfs/IPFSContext.test.tsxcomponents/brain/notifications/drop-reacted/NotificationDropReactedGroup.tsxcomponents/brain/notifications/subcomponents/NotificationHeader.tsxcomponents/common/OverlappingAvatars.tsxcomponents/common/image/useGatewayImageLoadState.tscomponents/ipfs/IPFSContext.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
__tests__/helpers/image.helpers.test.ts (1)
40-44: Test correctly verifies no re-resolution, but consider moving to separate describe block.The test validates the expected behavior of
getScaledResolvedImageUri. However, it's placed inside thedescribe("getScaledImageUri")block while testing a different function.♻️ Suggested organization
}); + +describe("getScaledResolvedImageUri", () => { + it("does not re-resolve already concrete urls", () => { + const url = "https://ipfs.io/ipfs/QmConcrete"; + expect(getScaledResolvedImageUri(url, ImageScale.W_AUTO_H_50)).toBe(url); + }); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/helpers/image.helpers.test.ts` around lines 40 - 44, The test asserting that a concrete IPFS URL is not re-resolved belongs to the getScaledResolvedImageUri behavior, not getScaledImageUri; move the it("does not re-resolve already concrete urls", ...) out of the describe("getScaledImageUri") block and into a new or existing describe("getScaledResolvedImageUri") block so tests are organized by the function under test (referencing function name getScaledResolvedImageUri and existing describe for getScaledImageUri to locate where to relocate it).helpers/image.helpers.ts (1)
53-55: Existing call sites now double-resolve URLs due to new internal call ingetScaledImageUri.The refactored
getScaledImageUrifunction (line 54) now callsresolveIpfsUrlSyncinternally, creating redundant resolution at three existing call sites:
WaveleaderboardDropRaters.tsx:67–71:resolveIpfsUrlSync(getScaledImageUri(...))WaveDropRatings.tsx:86–90:resolveIpfsUrlSync(getScaledImageUri(...))SingleWaveDropLog.tsx:104:getScaledImageUri(resolveIpfsUrlSync(...), ...)While functionally safe (resolution is idempotent for both IPFS and non-IPFS URLs), these patterns waste cycles and introduce inconsistency. Clean up by removing the outer
resolveIpfsUrlSynccalls in the first two files and the innerresolveIpfsUrlSynccall in the third.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helpers/image.helpers.ts` around lines 53 - 55, getScaledImageUri now calls resolveIpfsUrlSync internally, causing double-resolution at the three call sites; remove the redundant resolveIpfsUrlSync calls so each URL is resolved only once. Specifically: in WaveleaderboardDropRaters and WaveDropRatings remove the outer resolveIpfsUrlSync(...) wrapping getScaledImageUri(...), and in SingleWaveDropLog change the call to pass the raw URL into getScaledImageUri(...) (remove the inner resolveIpfsUrlSync there). Keep using getScaledImageUri(url, scale) and delete the extra resolveIpfsUrlSync invocations to avoid wasted work and inconsistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@__tests__/helpers/image.helpers.test.ts`:
- Around line 40-44: The test asserting that a concrete IPFS URL is not
re-resolved belongs to the getScaledResolvedImageUri behavior, not
getScaledImageUri; move the it("does not re-resolve already concrete urls", ...)
out of the describe("getScaledImageUri") block and into a new or existing
describe("getScaledResolvedImageUri") block so tests are organized by the
function under test (referencing function name getScaledResolvedImageUri and
existing describe for getScaledImageUri to locate where to relocate it).
In `@helpers/image.helpers.ts`:
- Around line 53-55: getScaledImageUri now calls resolveIpfsUrlSync internally,
causing double-resolution at the three call sites; remove the redundant
resolveIpfsUrlSync calls so each URL is resolved only once. Specifically: in
WaveleaderboardDropRaters and WaveDropRatings remove the outer
resolveIpfsUrlSync(...) wrapping getScaledImageUri(...), and in
SingleWaveDropLog change the call to pass the raw URL into
getScaledImageUri(...) (remove the inner resolveIpfsUrlSync there). Keep using
getScaledImageUri(url, scale) and delete the extra resolveIpfsUrlSync
invocations to avoid wasted work and inconsistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d706aba-0d30-4d75-b913-1759fc1f3b21
📒 Files selected for processing (9)
__tests__/components/ipfs/IPFSContext.test.tsx__tests__/helpers/image.helpers.test.tscomponents/brain/notifications/subcomponents/NotificationHeader.tsxcomponents/common/OverlappingAvatars.tsxcomponents/ipfs/IPFSContext.tsxhelpers/image.helpers.tsnext-env.typecheck.d.tspackage.jsontsconfig.typecheck.json
✅ Files skipped from review due to trivial changes (2)
- next-env.typecheck.d.ts
- tests/components/ipfs/IPFSContext.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- components/ipfs/IPFSContext.tsx
|
ragnep
approved these changes
Apr 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Chores