Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 32 minutes and 46 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. 📝 WalkthroughWalkthroughReplaces internal API proxy routes ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
coderabbitai
Bot
added
the
feature request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/utils/tokenMetadata.ts (1)
109-123: Normalize deserialized addresses before callinginfoToKey.Line 117 is the only key path here that skips
normalizeAddress(). If the data API returns checksum-casedchainId:addresskeys, later lookups miss and the corresponding market gets dropped.As per coding guidelines: All market/token/route identity checks must be chain-scoped using canonical identifiers (chainId + market.uniqueKey or chainId + address).♻️ Proposed fix
- if (Number.isInteger(chainId) && address) { - return [infoToKey(address, chainId), resolvedTokenInfo] satisfies [string, ResolvedTokenInfo]; + if (Number.isInteger(chainId) && address) { + return [infoToKey(normalizeAddress(address), chainId as SupportedNetworks), resolvedTokenInfo] satisfies [string, ResolvedTokenInfo]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/tokenMetadata.ts` around lines 109 - 123, deserializeResolvedTokenInfos currently calls infoToKey(address, chainId) with the raw address when splitting keys like "chainId:address", which can be checksum-cased and break later lookups; update the branch inside deserializeResolvedTokenInfos to normalize the address (using normalizeAddress) before calling infoToKey so keys are stored in the canonical, chain-scoped form; ensure you import/avail normalizeAddress and use it for the [chainIdRaw, address] path prior to infoToKey(address, chainId).src/hooks/queries/useMarketMetricsQuery.ts (1)
112-124: Wire the query cancellation signal into the new fetch path.This direct data-API version can spawn several page requests, but
queryFnnever passes TanStack Query'ssignaldown tofetch(). If the view unmounts or params change mid-flight, those requests keep running for no reason.♻️ Proposed fix
-const fetchMarketMetricsPage = async (params: MarketMetricsParams, limit: number, offset: number): Promise<MarketMetricsResponse> => { +const fetchMarketMetricsPage = async ( + params: MarketMetricsParams, + limit: number, + offset: number, + signal?: AbortSignal, +): Promise<MarketMetricsResponse> => { @@ - const response = await fetch(getMarketMetricsClientUrl(searchParams)); + const response = await fetch(getMarketMetricsClientUrl(searchParams), { signal }); @@ -const fetchAllMarketMetrics = async (params: MarketMetricsParams): Promise<MarketMetricsResponse> => { +const fetchAllMarketMetrics = async (params: MarketMetricsParams, signal?: AbortSignal): Promise<MarketMetricsResponse> => { @@ - const firstPage = await fetchMarketMetricsPage(params, PAGE_SIZE, 0); + const firstPage = await fetchMarketMetricsPage(params, PAGE_SIZE, 0, signal); @@ - pagePromises.push(fetchMarketMetricsPage(params, PAGE_SIZE, i * PAGE_SIZE)); + pagePromises.push(fetchMarketMetricsPage(params, PAGE_SIZE, i * PAGE_SIZE, signal)); @@ - queryFn: () => fetchAllMarketMetrics({ chainId, sortBy, sortOrder }), + queryFn: ({ signal }) => fetchAllMarketMetrics({ chainId, sortBy, sortOrder }, signal),Also applies to: 194-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/queries/useMarketMetricsQuery.ts` around lines 112 - 124, fetchMarketMetricsPage currently calls fetch(getMarketMetricsClientUrl(...)) without an AbortSignal; update its signature to accept an optional signal: (params: MarketMetricsParams, limit: number, offset: number, signal?: AbortSignal), and pass that signal into fetch as fetch(getMarketMetricsClientUrl(searchParams), { signal }); then update the caller (the queryFn that receives TanStack Query's signal) to forward its signal into fetchMarketMetricsPage so in-flight page requests are aborted when the query is cancelled; apply the identical change to the other page-fetching function in this module that mirrors lines ~194-198.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.local.example:
- Around line 60-63: The README/example env note incorrectly says
NEXT_PUBLIC_DATA_API_BASE_URL is production-only; change the comment in
.env.local.example to state that NEXT_PUBLIC_DATA_API_BASE_URL is required for
any environment that renders markets (not just production) because
useMarketMetricsQuery and tokenMetadata resolution in
src/hooks/queries/useMarketMetricsQuery.ts and src/utils/tokenMetadata.ts will
throw if it's missing; update the explanatory copy to instruct developers to set
a valid base URL for metrics and token metadata in all local/dev/staging
environments that work with markets.
In `@docs/TECHNICAL_OVERVIEW.md`:
- Line 193: The docs still mix responsibilities between the new
/v1/markets/metrics endpoint and the older Monarch backend: update all remaining
references that call out "Monarch API" and "5 min stale" (e.g., the lines that
currently mention Monach API / 5 min stale) to point to /v1/markets/metrics
where market metrics live, and ensure admin stats remain documented under
useMonarchTransactions (remove any phrasing on Line 435 implying the metrics
endpoint owns admin stats). Search for and replace inconsistent mentions (e.g.,
"Monarch API", "5 min stale", "admin stats" phrasing) so the page consistently
shows /v1/markets/metrics as the market metrics backend and
useMonarchTransactions as the owner of admin transaction stats.
In `@src/utils/tokenMetadata.ts`:
- Around line 276-315: The current fetchResolvedUnknownTokenInfosFromServer
logic swallows failed batch fetches (Promise.allSettled + skip) which loses
token metadata instead of falling back; update
fetchResolvedUnknownTokenInfosFromServer to (1) track which token inputs failed
per-batch when a response is rejected, (2) after Promise.allSettled, build a
list of unresolved token addresses (use the same TokenAddressInput shape and
dedupeTokenInputs helper) and call a shared RPC resolution routine (e.g., a new
resolveTokensViaRpcMulticall that performs chain-scoped multicalls keyed by
canonical chainId+address and returns a Map<string, ResolvedTokenInfo>), (3)
merge the server-resolved Map (from deserializeResolvedTokenInfos) with the
RPC-resolved Map, preferring server values but using RPC for missing entries,
and (4) if any required token remains unresolved after RPC, throw (fail closed)
instead of returning incomplete results; apply the same fix to the similar block
referenced around lines 318-328 so both paths use the RPC fallback and propagate
failures rather than silently omitting tokens.
---
Nitpick comments:
In `@src/hooks/queries/useMarketMetricsQuery.ts`:
- Around line 112-124: fetchMarketMetricsPage currently calls
fetch(getMarketMetricsClientUrl(...)) without an AbortSignal; update its
signature to accept an optional signal: (params: MarketMetricsParams, limit:
number, offset: number, signal?: AbortSignal), and pass that signal into fetch
as fetch(getMarketMetricsClientUrl(searchParams), { signal }); then update the
caller (the queryFn that receives TanStack Query's signal) to forward its signal
into fetchMarketMetricsPage so in-flight page requests are aborted when the
query is cancelled; apply the identical change to the other page-fetching
function in this module that mirrors lines ~194-198.
In `@src/utils/tokenMetadata.ts`:
- Around line 109-123: deserializeResolvedTokenInfos currently calls
infoToKey(address, chainId) with the raw address when splitting keys like
"chainId:address", which can be checksum-cased and break later lookups; update
the branch inside deserializeResolvedTokenInfos to normalize the address (using
normalizeAddress) before calling infoToKey so keys are stored in the canonical,
chain-scoped form; ensure you import/avail normalizeAddress and use it for the
[chainIdRaw, address] path prior to infoToKey(address, chainId).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 690276d7-869b-4f4c-9974-5f14701eda59
📒 Files selected for processing (11)
.env.local.exampleapp/api/monarch/metrics/route.tsapp/api/monarch/utils.tsapp/api/token-metadata/route.tsdocs/TECHNICAL_OVERVIEW.mdnext.config.jssrc/features/markets/components/filters/asset-filter.tsxsrc/hooks/queries/useMarketMetricsQuery.tssrc/server/token-metadata-cache.tssrc/utils/tokenMetadata.tssrc/utils/urls.ts
💤 Files with no reviewable changes (4)
- app/api/token-metadata/route.ts
- src/server/token-metadata-cache.ts
- app/api/monarch/metrics/route.ts
- app/api/monarch/utils.ts
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
1 participant
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
Refactor
Chores