feat: kleros identity badge#535
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Kleros Scout address tag types and fetch, a React Query hook, KlerosTagBadge UI, AccountActionsPopover extraLinks support, and wires tags into AccountIdentity, market-detail tables, and oracle UI; docs updated. ChangesKleros Address Tags Feature
Sequence Diagram(s): sequenceDiagram
participant Client
participant useKlerosAddressTagsQuery
participant fetchKlerosAddressTags
participant KlerosScoutAPI
participant AccountIdentity
Client->>useKlerosAddressTagsQuery: request tags for addresses
useKlerosAddressTagsQuery->>fetchKlerosAddressTags: POST normalized addresses
fetchKlerosAddressTags->>KlerosScoutAPI: HTTP POST /api/kleros/address-tags
KlerosScoutAPI-->>fetchKlerosAddressTags: tags response
fetchKlerosAddressTags-->>useKlerosAddressTagsQuery: tags map
useKlerosAddressTagsQuery-->>AccountIdentity: provide klerosTag
AccountIdentity->>AccountIdentity: render KlerosTagBadge when present
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Code Review
This pull request integrates the Kleros Scout API to display address tags and project metadata for Ethereum addresses throughout the application. It introduces a new API route for proxying requests to Kleros, a custom React Query hook for data fetching, and UI enhancements to components like AccountIdentity and AccountActionsPopover to render these tags. Review feedback identifies a potential type-mismatch issue in the API route's chain ID validation and recommends using Next.js client-side routing instead of full page reloads for internal links within the account actions menu.
| } | ||
|
|
||
| function parseScoutTag(address: KlerosAddressTag['address'], chainId: number, value: unknown): KlerosAddressTag | null { | ||
| if (!isRecord(value) || readText(value.chain_id) !== String(chainId)) { |
There was a problem hiding this comment.
The readText helper strictly checks for a string type. If the Scout API returns chain_id as a number (which is common for EVM chain IDs), this check will fail and return null, causing valid tags to be ignored. It's safer to normalize the value to a string before comparison.
| if (!isRecord(value) || readText(value.chain_id) !== String(chainId)) { | |
| if (!isRecord(value) || String(value.chain_id) !== String(chainId)) { |
|
|
||
| import { useCallback, type ReactNode } from 'react'; | ||
| import { useRouter } from 'next/navigation'; | ||
| import { DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem } from '@/components/ui/dropdown-menu'; |
There was a problem hiding this comment.
Since we need to handle internal navigation for app-relative links (like the profile or vault links) without triggering a full page reload, we should restore the useRouter import.
| import { DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem } from '@/components/ui/dropdown-menu'; | |
| import { useRouter } from 'next/navigation'; | |
| import { DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem } from '@/components/ui/dropdown-menu'; |
| profileLabel = 'View Portfolio', | ||
| }: AccountActionsPopoverProps) { | ||
| const router = useRouter(); | ||
| const toast = useStyledToast(); |
| const handleOpenLink = useCallback((href: string) => { | ||
| if (isExternalHref(href)) { | ||
| window.open(href, '_blank', 'noopener,noreferrer'); | ||
| return; | ||
| } | ||
|
|
||
| if (isAppRelativeHref(href)) { | ||
| window.location.assign(href); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Using window.location.assign for internal app links causes a full page reload, which breaks the Single Page Application (SPA) experience and is less efficient than using Next.js's client-side routing. Use router.push for app-relative links instead.
| const handleOpenLink = useCallback((href: string) => { | |
| if (isExternalHref(href)) { | |
| window.open(href, '_blank', 'noopener,noreferrer'); | |
| return; | |
| } | |
| if (isAppRelativeHref(href)) { | |
| window.location.assign(href); | |
| } | |
| }, []); | |
| const handleOpenLink = useCallback((href: string) => { | |
| if (isExternalHref(href)) { | |
| window.open(href, '_blank', 'noopener,noreferrer'); | |
| return; | |
| } | |
| if (isAppRelativeHref(href)) { | |
| router.push(href); | |
| } | |
| }, [router]); |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/kleros/address-tags/route.ts`:
- Around line 129-137: The POST fetch to KLEROS_SCOUT_ADDRESS_TAGS_URL currently
has no timeout and can hang; wrap the call with an AbortController, pass
controller.signal into the fetch options (alongside existing
headers/body/cache), start a setTimeout that calls controller.abort() after a
sensible timeout (e.g., a few seconds), and clear that timer after fetch
resolves or rejects; update the error handling around the fetch/response (where
response is used) to treat an AbortError as a timeout (returning/throwing an
appropriate 504/timeout error). Ensure you reference the existing symbols
KLEROS_SCOUT_ADDRESS_TAGS_URL, normalizedAddresses, chainId, and the local
response variable when implementing this change.
In `@src/components/shared/kleros-tag-badge.tsx`:
- Around line 47-53: The tooltip anchor is a plain <span> so keyboard users
can’t focus it; update the KlerosTagBadge component (the element rendering the
wrapper with className, aria-label, KlerosBadgeLogo and label) to be
keyboard-focusable by replacing the non-focusable span with an
interactive/focusable element (e.g., a <button> or a <span> with tabIndex={0}
and role="button"), preserve the existing aria-label, and ensure the tooltip
library receives focus events (or add an onKeyDown handler to open the tooltip
on Enter/Space) so keyboard users can trigger and read the tooltip content.
🪄 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: e7d85201-8063-4d07-a1e4-968306922ee3
⛔ Files ignored due to path filters (1)
public/imgs/kleros-badge.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
app/api/kleros/address-tags/route.tsdocs/TECHNICAL_OVERVIEW.mdsrc/components/shared/account-actions-popover.tsxsrc/components/shared/account-identity.tsxsrc/components/shared/kleros-tag-badge.tsxsrc/data-sources/kleros/address-tags.tssrc/features/market-detail/components/borrowers-table.tsxsrc/features/market-detail/components/suppliers-table.tsxsrc/features/markets/components/oracle/MarketOracle/OracleTypeInfo.tsxsrc/hooks/queries/useKlerosAddressTagsQuery.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/data-sources/kleros/address-tags.ts`:
- Line 159: The slice silently drops addresses beyond MAX_ADDRESSES_PER_REQUEST;
update the code around normalizeKlerosAddressList and normalizedAddresses to
detect when addresses.length > MAX_ADDRESSES_PER_REQUEST and emit a clear
warning (e.g., console.warn or the module logger) stating how many addresses
were truncated and which input was trimmed, then proceed to slice; alternatively
(or additionally) add a JSDoc on the containing function mentioning the
MAX_ADDRESSES_PER_REQUEST limit so callers are aware.
🪄 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: db95ad1e-78f3-4323-b90f-7f706c1fcc64
📒 Files selected for processing (6)
docs/TECHNICAL_OVERVIEW.mdsrc/components/shared/account-identity.tsxsrc/components/shared/kleros-tag-badge.tsxsrc/data-sources/kleros/address-tags.tssrc/features/market-detail/components/borrowers-table.tsxsrc/features/market-detail/components/suppliers-table.tsx
✅ Files skipped from review due to trivial changes (1)
- docs/TECHNICAL_OVERVIEW.md
| chainId, | ||
| signal, | ||
| }: FetchKlerosAddressTagsParams): Promise<KlerosAddressTagsByKey> { | ||
| const normalizedAddresses = normalizeKlerosAddressList(addresses).slice(0, MAX_ADDRESSES_PER_REQUEST); |
There was a problem hiding this comment.
Warn when addresses are silently dropped.
The slice silently truncates to 50 addresses. If a caller passes 100 addresses, the last 50 are ignored with no indication. Add a warning or document this limit in a JSDoc comment.
🔊 Add warning when truncating
const normalizedAddresses = normalizeKlerosAddressList(addresses).slice(0, MAX_ADDRESSES_PER_REQUEST);
+
+if (normalizedAddresses.length < addresses.length) {
+ console.warn(`[kleros-address-tags] Truncated to ${MAX_ADDRESSES_PER_REQUEST} addresses (received ${addresses.length})`);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const normalizedAddresses = normalizeKlerosAddressList(addresses).slice(0, MAX_ADDRESSES_PER_REQUEST); | |
| const normalizedAddresses = normalizeKlerosAddressList(addresses).slice(0, MAX_ADDRESSES_PER_REQUEST); | |
| if (normalizedAddresses.length < addresses.length) { | |
| console.warn(`[kleros-address-tags] Truncated to ${MAX_ADDRESSES_PER_REQUEST} addresses (received ${addresses.length})`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/data-sources/kleros/address-tags.ts` at line 159, The slice silently
drops addresses beyond MAX_ADDRESSES_PER_REQUEST; update the code around
normalizeKlerosAddressList and normalizedAddresses to detect when
addresses.length > MAX_ADDRESSES_PER_REQUEST and emit a clear warning (e.g.,
console.warn or the module logger) stating how many addresses were truncated and
which input was trimmed, then proceed to slice; alternatively (or additionally)
add a JSDoc on the containing function mentioning the MAX_ADDRESSES_PER_REQUEST
limit so callers are aware.
Summary by CodeRabbit
New Features
Documentation
Closes #536