feat: stats and leaderboard renewal #34
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@hyochan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 49 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a leaderboards feature with tier filtering and top-tier carousel, introduces monthly contribution fetching and charts for stats with an endDate selector, adds tier-related API endpoints, extends GitHub service types and date-range parameters, adjusts layouts/styles, and updates localization and documentation. Changes
Sequence DiagramssequenceDiagram
actor User
participant Page as Stats Page\n[login]/page.tsx
participant GithubService as GithubService
participant GitHub as GitHub GraphQL
participant Scouter as Scouter Component
participant MonthPicker as MonthPicker
participant StatsChart as StatsChart
User->>Page: Open stats (optional endDate)
Page->>GithubService: getDoobooStats(login, startDate)
Page->>GithubService: getMonthlyContribution(login, startDate)
par Parallel GitHub queries
GithubService->>GitHub: Query aggregated stats
GithubService->>GitHub: Query monthly contributions
end
GitHub-->>GithubService: Responses
GithubService-->>Page: Stats + MonthlyContribution[]
Page->>Scouter: Render with statsWithMonthly & endDate
Scouter->>StatsChart: supply monthlyContributions
Scouter->>MonthPicker: supply value (endDate)
User->>MonthPicker: Select month → onChangeAction
MonthPicker->>Scouter: onChangeAction(newEndDate)
Scouter->>Page: push endDate to URL (router)
StatsChart-->>User: Render interactive chart
sequenceDiagram
actor User
participant Page as Leaderboards Page
participant TopTier as TopTierUsers
participant List as GithubUserList
participant API_Top as /api/top-tier-users
participant API_Tier as /api/users-by-tier
participant Supabase as Supabase
User->>Page: Visit leaderboards
Page->>TopTier: mount
TopTier->>API_Top: GET
API_Top->>Supabase: query top 5 plugin users
Supabase-->>API_Top: results
API_Top-->>TopTier: PluginUser[]
TopTier-->>User: show carousel
Page->>List: render recent list (initial data)
User->>List: click tier button (e.g., Gold)
List->>API_Tier: GET ?tier=Gold
API_Tier->>Supabase: query users within tier score range
Supabase-->>API_Tier: filtered users
API_Tier-->>List: PluginUser[]
List-->>User: display tier users
User->>List: click "All"
List-->>User: reset to recent data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Summary of ChangesHello @hyochan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant upgrade to the application's user statistics and leaderboard functionalities. It introduces advanced data visualization tools and flexible filtering options, allowing users to explore performance metrics with greater detail and interactivity. The changes are supported by robust backend enhancements for data retrieval and a refined user interface that adapts seamlessly to various devices. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive pull request that significantly revamps the stats and leaderboard functionality. The introduction of interactive charts with recharts, tier-based filtering on the leaderboard, and a date picker for stats are all excellent feature additions that greatly enhance the user experience. The backend changes, including new API endpoints and updates to the GitHub data fetching logic to support date ranges and monthly contributions, are well-implemented. I particularly appreciate the thoughtful approach to caching, where you only update the database for default data fetches, preventing filtered views from overwriting the main cache. My feedback is focused on improving code consistency by adhering to the project's own styling guidelines, enhancing type safety, and removing some minor code duplications and magic numbers. Overall, this is a high-quality contribution.
| #### Brand/Accent Colors | ||
| ```tsx | ||
| // Primary brand color (blue) | ||
| className="bg-brand" // #4190EB |
There was a problem hiding this comment.
It's great that you've added this coding guidelines file! To improve clarity, you could add a comment explaining the purpose of the bg-brand color, similar to how you've done for other colors like "Primary buttons" or "Status colors". This helps developers understand when to use it.
| className="bg-brand" // #4190EB | |
| // Primary brand color (blue) - used for primary actions and highlights | |
| className="bg-brand" // #4190EB |
| stargazers { | ||
| totalCount | ||
| } |
There was a problem hiding this comment.
Removing the owner field from this GraphQL query is a good optimization since it doesn't appear to be used. This helps to reduce the payload size from the GitHub API.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/[lang]/(home)/SectionFooter.tsx (1)
35-39: Remove stray quote in Tailwind class string
'md:p-0"'includes an extra"at the end, so the class becomesmd:p-0"and Tailwind likely won’t apply the intendedmd:p-0utility. Drop the trailing quote inside the string.- 'flex flex-col justify-center items-center', - 'md:p-0"', + 'flex flex-col justify-center items-center', + 'md:p-0',app/[lang]/recent-list/GithubUserList.tsx (1)
144-144: Reference comparison inincludes()will not filter duplicates correctly.
data.includes(el)performs reference equality, not value comparison. Sinceusersfrom the API are new objects, they will never match existing objects indata, causing duplicates when properties match.- const filteredUsers = users.filter((el) => !data.includes(el)); + const filteredUsers = users.filter( + (el) => !data.some((existing) => existing.githubId === el.githubId), + );pages/api/github-trophies.ts (1)
25-68: Wrap case block in braces to scope the variable declaration.The static analysis correctly identifies that
const startDatedeclared inside acasewithout block scope can be accidentally accessed from other switch clauses if they're added later.switch (method) { - case 'GET': - const startDate = <string>req.query.startDate; + case 'GET': { + const startDate = Array.isArray(req.query.startDate) + ? req.query.startDate[0] + : <string>req.query.startDate; assert(login, common.badRequest); try { const stats = await getDoobooStats({ login: login.toLocaleLowerCase(), lang: locale, startDate, }); if (!stats) { res.status(404).json({message: common.notFound}); return; } track('github-trophies', undefined, { language: locale, user_id: login, extra: {login, lang: locale}, }); const trophies = stats.pluginTrophies.filter( (el) => el.score > LOWEST_TROPHIES_SCORE, ); if (trophies.length === 0) { res.status(404).json({message: common.notFound}); return; } const trophySvg = renderGithubTrophies(trophies); uploadTrophiesSvg(login, trophySvg); res.setHeader('Content-Type', 'image/svg+xml'); res.send(trophySvg); } catch (err) { res.status(500).send(err); } break; + } default: res.status(404).json({message: common.notFound}); }pages/api/github-stats-advanced.ts (1)
23-55: Wrap case block in braces to scope the variable declaration.Same issue as in
github-trophies.ts- theconst startDatedeclaration inside acasewithout block scope can be accessed from other switch clauses.switch (method) { - case 'GET': - const startDate = <string>req.query.startDate; + case 'GET': { + const startDate = Array.isArray(req.query.startDate) + ? req.query.startDate[0] + : <string>req.query.startDate; assert(login, common.badRequest); try { const stats = await getDoobooStats({ login: login.toLocaleLowerCase(), lang: locale, startDate, }); if (!stats) { res.status(404).json({message: common.notFound}); return; } const {file} = await generateGithubSVG(login, stats, true); track('github-stats-advanced', undefined, { language: locale, user_id: login, extra: {login, lang: locale}, }); res.setHeader('Content-Type', 'image/svg+xml'); res.send(file); } catch (err) { res.status(500).send(err); } break; + } default: res.status(404).json({message: common.notFound}); }
🧹 Nitpick comments (17)
styles/root.css (1)
5-12: Consider using Tailwind's built-inanimate-spinutility.Tailwind CSS already provides an
animate-spinutility that creates the same rotation animation. Unless you need custom timing or behavior, the built-in utility is preferable for consistency and maintainability.If the default Tailwind animation suffices, you can remove this custom keyframes definition and use
animate-spindirectly in your components. If you need custom timing, consider extending Tailwind's animation configuration intailwind.config.jsinstead:-@keyframes spin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } -}Then in
tailwind.config.js:module.exports = { theme: { extend: { animation: { 'spin-slow': 'spin 3s linear infinite', } } } }app/[lang]/recent-list/GithubUserList.tsx (1)
50-71: Consider handling non-OK HTTP responses.The fetch call doesn't check
response.okbefore parsing JSON. A 4xx/5xx response would still attempt to parse and could lead to confusing behavior ifresult.usersis undefined.const handleTierSelect = useCallback(async (tier: Tier | null) => { setSelectedTier(tier); if (!tier) { setTierData([]); return; } setIsLoadingTier(true); try { const response = await fetch(`/api/users-by-tier?tier=${tier}`); + if (!response.ok) { + throw new Error(`HTTP error: ${response.status}`); + } const result = await response.json(); if (result.users) { setTierData(result.users); } } catch (error) { console.error('Failed to fetch tier users:', error); setTierData([]); } finally { setIsLoadingTier(false); } }, []);pages/api/top-tier-users.ts (1)
37-42: Consider handling potential Supabase query errors.The
userPluginsquery doesn't check for errors. While it gracefully falls back to an empty array, logging errors would help with debugging production issues.- const {data: userPlugins}: {data: UserPluginRow[] | null} = await supabase + const {data: userPlugins, error}: {data: UserPluginRow[] | null; error: unknown} = await supabase .from('user_plugins') .select('*') .match({plugin_id: 'dooboo-github'}) .order('score', {ascending: false}) .limit(5); + + if (error) { + console.error('Failed to fetch user plugins:', error); + }pages/api/users-by-tier.ts (2)
24-29: Handle potential array query parameter.When
tieris passed multiple times (e.g.,?tier=Gold&tier=Silver),query.tierbecomes an array. Casting directly tostringwould result in unexpected behavior.- const tier = query.tier as string; + const tier = Array.isArray(query.tier) ? query.tier[0] : query.tier; if (!tier) { res.status(400).send({message: 'Tier is required'}); return; }
64-71: Consider adding error handling for the database query.The query result doesn't check for errors. If the database call fails,
userPluginscould be null due to an error, but the code would silently return an empty array.- const {data: userPlugins}: {data: UserPluginRow[] | null} = await supabase + const {data: userPlugins, error}: {data: UserPluginRow[] | null; error: any} = await supabase .from('user_plugins') .select('*') .match({plugin_id: 'dooboo-github'}) .gte('score', minScore) .lte('score', maxScore) .order('score', {ascending: false}) .limit(50); + + if (error) { + res.status(500).send({message: 'Failed to fetch users'}); + return; + }server/services/githubService.ts (1)
275-327: Consider adding a delay between batches for rate limiting.The batching approach (size 3) helps with rate limiting, but there's no delay between batches. GitHub's GraphQL API has rate limits, and rapid sequential requests might still hit them.
// Fetch stats for each month in parallel (batch of 3 to avoid rate limiting) const batchSize = 3; for (let i = 0; i < months.length; i += batchSize) { const batch = months.slice(i, i + batchSize); const results = await Promise.all( batch.map(async (month) => { try { const {commits, pullRequests, reviews} = await fetchGithubMonthlyContributions(login, month); return { month, commits, pullRequests, reviews, total: commits + pullRequests + reviews, }; } catch { return {month, commits: 0, pullRequests: 0, reviews: 0, total: 0}; } }), ); monthlyContributions.push(...results); + + // Add small delay between batches to respect rate limits + if (i + batchSize < months.length) { + await new Promise((resolve) => setTimeout(resolve, 100)); + } }pages/api/github-stats.ts (2)
39-46: Consider validating startDate format.The startDate parameter is extracted from the query but not validated for the expected YYYY-MM format. While downstream services may handle invalid formats, adding validation at the API boundary would provide clearer error messages and prevent unnecessary processing.
Consider adding format validation:
const startDate = req.query.startDate as string | undefined; +if (startDate && !/^\d{4}-\d{2}$/.test(startDate)) { + res.status(400).json({message: 'Invalid startDate format. Expected YYYY-MM'}); + return; +} assert(loginParam, common.badRequest);
71-78: Consider validating startDate format.Similar to the GET endpoint, the POST endpoint should validate the startDate format before passing it to downstream services.
Apply the same validation pattern:
const startDateBody = req.body.startDate as string | undefined; +if (startDateBody && !/^\d{4}-\d{2}$/.test(startDateBody)) { + res.status(400).json({message: 'Invalid startDate format. Expected YYYY-MM'}); + return; +} assert(loginBody, common.badRequest);app/[lang]/stats/[login]/Scouter/StatsDetails/StatsRadar.tsx (1)
39-57: Consider responsive dimensions for the radar chart.The chart uses fixed dimensions (280×250). While this may work for the current layout, consider making the chart responsive to container width for better adaptability across screen sizes.
If responsiveness is desired, you could use ResponsiveContainer from recharts:
return ( <div className="mt-6"> - <RadarChart - width={280} - height={250} + <ResponsiveContainer width="100%" height={250}> + <RadarChart - data={radarData} - cx="50%" - cy="50%" - outerRadius="70%" - > + data={radarData} + cx="50%" + cy="50%" + outerRadius="70%" + > - <PolarGrid stroke="#444" /> - <PolarAngleAxis dataKey="stat" tick={{fill: '#888', fontSize: 12}} /> - <Radar - name="Score" - dataKey="value" - stroke="#3b82f6" - fill="#3b82f6" - fillOpacity={0.4} - strokeWidth={2} - /> - </RadarChart> + <PolarGrid stroke="#444" /> + <PolarAngleAxis dataKey="stat" tick={{fill: '#888', fontSize: 12}} /> + <Radar + name="Score" + dataKey="value" + stroke="#3b82f6" + fill="#3b82f6" + fillOpacity={0.4} + strokeWidth={2} + /> + </RadarChart> + </ResponsiveContainer> </div> );And add the import:
-import {RadarChart, PolarGrid, PolarAngleAxis, Radar} from 'recharts'; +import {RadarChart, PolarGrid, PolarAngleAxis, Radar, ResponsiveContainer} from 'recharts';app/[lang]/recent-list/TopTierUsers.tsx (2)
22-38: Consider showing error state to users.The error is logged to console but users see only a loading skeleton that never completes. Consider showing an error message or gracefully degrading to an empty state.
Add error state:
const [topTierUsers, setTopTierUsers] = useState<UserListItem[]>([]); const [isLoading, setIsLoading] = useState(true); +const [error, setError] = useState(false); useEffect(() => { const fetchTopUsers = async () => { try { const response = await fetch('/api/top-tier-users'); const data = await response.json(); if (data.users) { setTopTierUsers(data.users); } } catch (error) { console.error('Failed to fetch top tier users:', error); + setError(true); } finally { setIsLoading(false); } }; fetchTopUsers(); }, []); +if (error) { + return <div />; +} + if (isLoading) {
100-100: Type cast may be unsafe.The tierName is cast to Tier without validation. If the API returns an unexpected tier name, getTierSvg will fall back to Iron tier, but the type system won't catch this mismatch.
Consider validating the tier or making getTierSvg more defensive:
-src={getTierSvg(user.tierName as Tier)} +src={getTierSvg(user.tierName as Tier) || '/assets/tier_iron.svg'}Or add runtime validation if stricter type safety is desired.
app/[lang]/stats/[login]/page.tsx (1)
47-47: Parameter naming inconsistency: endDate passed as startDate.The variable
endDateis passed togetDoobooStatsas thestartDateparameter. Based on the context, it appearsendDaterepresents the end of a time range, but it's being used as a filter point. This naming mismatch could cause confusion.Consider adding a clarifying comment or renaming for consistency:
const [stats, monthlyContributions] = await Promise.all([ - getDoobooStats({login, lang, startDate: endDate}), + getDoobooStats({login, lang, startDate: endDate}), // endDate filters stats up to this month getMonthlyContribution({login, startDate}), ]);app/[lang]/stats/[login]/MonthPicker.tsx (3)
84-102: Duplicate month names array.The
monthNamesarray duplicatesMONTHSconstant defined at lines 23-27. Reuse the existing constant for consistency and maintainability.const formatDisplay = () => { if (!value) { return t.selectPeriod; } const [year, month] = value.split('-'); - const monthNames = [ - 'Jan', - 'Feb', - 'Mar', - 'Apr', - 'May', - 'Jun', - 'Jul', - 'Aug', - 'Sep', - 'Oct', - 'Nov', - 'Dec', - ]; - const monthName = monthNames[parseInt(month) - 1]; + const monthName = MONTHS[parseInt(month) - 1]; // Show year only if it's not the current year if (parseInt(year) === currentYear) { return monthName; } return `${monthName} ${year}`; };
36-42:viewYeardoesn't sync whenvalueprop changes externally.The
viewYearstate is initialized fromvaluebut never updated ifvaluechanges after mount (e.g., parent resets it). Consider adding an effect to syncviewYearwhenvaluechanges significantly.+ useEffect(() => { + if (value) { + setViewYear(parseInt(value.split('-')[0])); + } + }, [value]);
164-172: Consider adding accessibility attributes for the dropdown.The dropdown lacks
aria-expanded,aria-haspopup, and keyboard navigation support (Escape to close). This could impact accessibility compliance.<button type="button" onClick={() => setIsOpen(!isOpen)} + aria-expanded={isOpen} + aria-haspopup="listbox" className={clsx(app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (2)
83-101: Consider extracting month formatting to a shared utility.The
formatMonthfunction duplicates the same month names array found inMonthPicker.tsx. Consider extracting this to a shared utility for consistency and reusability.Create a shared utility:
// src/utils/dateUtils.ts export const MONTH_NAMES = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec'] as const; export function formatMonth(month: string): string { const [, monthNum] = month.split('-'); return MONTH_NAMES[parseInt(monthNum, 10) - 1]; }
173-208: Conditional rendering of<Line>components may cause legend inconsistencies.When lines are hidden via
isLineVisible(), the corresponding legend entries also disappear, which might confuse users about the toggle behavior. Consider keeping all lines rendered but usingstrokeOpacity={0}for hidden lines to preserve legend entries.- {isLineVisible('commits') && ( - <Line - ... - strokeOpacity={getLineOpacity('commits')} - /> - )} + <Line + ... + strokeOpacity={isLineVisible('commits') ? getLineOpacity('commits') : 0} + dot={isLineVisible('commits') ? {fill: '#22c55e', strokeWidth: 2, r: 3} : false} + />This preserves the legend for all series while visually hiding inactive ones.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
CLAUDE.md(1 hunks)app/[lang]/(common)/Header/index.tsx(1 hunks)app/[lang]/(home)/SectionFooter.tsx(1 hunks)app/[lang]/layout.tsx(1 hunks)app/[lang]/recent-list/GithubUserList.tsx(4 hunks)app/[lang]/recent-list/TopTierUsers.tsx(1 hunks)app/[lang]/recent-list/page.tsx(3 hunks)app/[lang]/stats/Container.tsx(1 hunks)app/[lang]/stats/[login]/MonthPicker.tsx(1 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx(5 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/SectionPeople.tsx(2 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx(1 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsRadar.tsx(1 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/index.tsx(2 hunks)app/[lang]/stats/[login]/Scouter/StatsHeader.tsx(1 hunks)app/[lang]/stats/[login]/Scouter/index.tsx(3 hunks)app/[lang]/stats/[login]/SearchTextInput.tsx(4 hunks)app/[lang]/stats/[login]/page.tsx(3 hunks)app/[lang]/stats/page.tsx(1 hunks)app/[lang]/styles.module.css(2 hunks)locales/en.json(2 hunks)locales/ko.json(2 hunks)package.json(1 hunks)pages/api/github-stats-advanced.ts(1 hunks)pages/api/github-stats.ts(2 hunks)pages/api/github-trophies.ts(1 hunks)pages/api/top-tier-users.ts(1 hunks)pages/api/users-by-tier.ts(1 hunks)server/plugins/types/UserGraph.ts(1 hunks)server/services/githubService.ts(12 hunks)styles/output.css(37 hunks)styles/root.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (17)
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionPeople.tsx (1)
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsRadar.tsx (1)
StatsRadar(15-60)
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsRadar.tsx (2)
server/plugins/index.ts (1)
PluginStats(57-64)src/localization.ts (1)
Translates(11-11)
app/[lang]/recent-list/TopTierUsers.tsx (3)
src/fetches/recentList.ts (1)
UserListItem(5-12)src/utils/functions.ts (1)
getTierSvg(99-115)app/[lang]/recent-list/TierRowItem.tsx (1)
Tier(6-14)
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
server/services/githubService.ts (1)
MonthlyContribution(365-371)
app/[lang]/stats/[login]/MonthPicker.tsx (1)
src/localization.ts (1)
Translates(11-11)
app/[lang]/stats/[login]/SearchTextInput.tsx (1)
src/hooks/useSearchHistory.ts (1)
useSearchHistory(26-65)
app/[lang]/stats/[login]/Scouter/index.tsx (3)
server/services/githubService.ts (2)
DoobooStatsResponse(373-388)MonthlyContribution(365-371)src/fetches/github.ts (1)
StatsInfo(41-48)app/[lang]/stats/[login]/Scouter/StatsDetails/index.tsx (1)
StatsDetails(19-45)
pages/api/github-stats-advanced.ts (1)
server/services/githubService.ts (1)
getDoobooStats(631-809)
pages/api/github-trophies.ts (2)
src/utils/assert.ts (1)
assert(7-14)server/services/githubService.ts (1)
getDoobooStats(631-809)
pages/api/users-by-tier.ts (4)
pages/api/top-tier-users.ts (1)
handler(11-62)src/utils/functions.ts (1)
PluginUser(43-58)server/supabaseClient.ts (1)
getSupabaseClient(9-17)src/types/types.ts (1)
UserPluginRow(10-10)
app/[lang]/recent-list/page.tsx (2)
app/[lang]/recent-list/TopTierUsers.tsx (1)
TopTierUsers(18-114)app/[lang]/(common)/GreatFrontEnd.tsx (1)
GreatFrontEnd(16-43)
server/services/githubService.ts (2)
server/plugins/types/UserGraph.ts (1)
UserGraph(1-31)src/types/supabase.ts (1)
Json(1-7)
pages/api/github-stats.ts (2)
src/utils/assert.ts (1)
assert(7-14)server/services/githubService.ts (1)
getDoobooStats(631-809)
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (3)
app/[lang]/stats/[login]/Scouter/index.tsx (2)
statNames(18-25)StatsWithMonthly(27-29)app/[lang]/stats/[login]/MonthPicker.tsx (1)
MonthPicker(29-265)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
StatsChart(24-214)
app/[lang]/stats/[login]/Scouter/StatsDetails/index.tsx (2)
app/[lang]/stats/[login]/Scouter/index.tsx (2)
ScouterProps(31-35)StatName(37-37)app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (1)
SectionDooboo(190-206)
app/[lang]/stats/[login]/page.tsx (4)
src/i18n.ts (1)
Locale(6-6)src/localization.ts (1)
getTranslates(17-23)server/services/githubService.ts (2)
getDoobooStats(631-809)getMonthlyContribution(812-832)app/[lang]/stats/[login]/Scouter/index.tsx (1)
Scouter(39-59)
app/[lang]/recent-list/GithubUserList.tsx (3)
app/[lang]/recent-list/TierRowItem.tsx (1)
Tier(6-14)src/fetches/recentList.ts (1)
UserListItem(5-12)src/utils/functions.ts (1)
getTierSvg(99-115)
🪛 Biome (2.1.2)
pages/api/github-stats-advanced.ts
[error] 25-25: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
pages/api/github-trophies.ts
[error] 27-27: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
styles/output.css
[error] 1508-1508: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
min-width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🪛 GitHub Actions: CI
app/[lang]/stats/[login]/SearchTextInput.tsx
[error] 37-37: ESLint: Calling setState synchronously within an effect. Avoid calling setState directly within an effect.
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx
[error] 32-32: ESLint: Calling setState synchronously within an effect. Avoid calling setState directly within an effect.
🔇 Additional comments (44)
app/[lang]/(home)/SectionFooter.tsx (1)
84-86: Designer link update to GitHub profile looks goodPointing the “hyochan” designer credit to the GitHub profile is correct and consistent with the project’s main CTA link.
server/plugins/types/UserGraph.ts (1)
84-105: Contribution calendar typing looks sound; confirm subset matches your data shapeThe new
contributionCalendarfield and theContributionCalendar/ContributionWeek/ContributionDayinterfaces are consistent and structurally sane: they’re backward‑compatible (optional field onContributionsCollection) and minimal but sufficient for typical calendar and chart use.The only thing to double‑check is that this subset (
totalContributions,weeks[].firstDay,weeks[].contributionDays[].{contributionCount,date}) matches exactly what your GitHub data layer (or API client) returns and what downstream chart components expect, especially aroundstringvsDatehandling and the possibility ofcontributionCalendarbeingundefined.app/[lang]/stats/[login]/SearchTextInput.tsx (4)
40-44: Verify the need forrouter.refresh()afterrouter.push().Calling
router.refresh()immediately afterrouter.push()may be redundant—navigating to a new route should already fetch fresh server component data. If you're trying to invalidate the router cache, consider whetherrouter.push()alone suffices, or userouter.replace()with cache invalidation options.
103-113: Ensure@keyframes spinis defined in global styles.The inline spinner relies on
animation: 'spin 1s linear infinite', but@keyframes spinmust be defined in your global CSS for this animation to work. Verify this keyframe exists, or consider using Tailwind's built-inanimate-spinclass instead:- <div - style={{ - width: '20px', - height: '20px', - border: '2px solid #BEBEBE', - borderTopColor: '#4190EB', - borderRadius: '50%', - animation: 'spin 1s linear infinite', - }} - /> + <div + className="w-5 h-5 border-2 border-[#BEBEBE] border-t-[#4190EB] rounded-full animate-spin" + />This approach uses Tailwind's
animate-spinutility which includes the necessary keyframes.
29-33: LGTM on language extraction.The pattern correctly extracts the language segment from the pathname with a sensible fallback to
'en'. This aligns with Next.js i18n routing conventions.
92-101: Good accessible button implementation.The native button with proper
type="submit", disabled state handling, and visual feedback is well-implemented.app/[lang]/stats/page.tsx (1)
22-22: LGTM!Removing the trailing period from the hint text improves the UI polish and follows common UI conventions for placeholder/hint text.
app/[lang]/layout.tsx (1)
33-36: LGTM! Proper flexbox overflow prevention.The addition of
min-w-0on flex containers is a well-known flexbox pattern to prevent flex items from overflowing. By default, flex items havemin-width: auto, which can cause content to overflow its container. Settingmin-w-0allows the flex item to shrink below its content size. Theoverflow-x-hiddenprevents unwanted horizontal scrolling, andw-fullensures proper width distribution.These changes align with similar layout constraint updates across other components in this PR.
app/[lang]/stats/[login]/Scouter/StatsHeader.tsx (1)
113-114: LGTM! Consistent width constraint pattern.The addition of explicit width constraints (
w-full min-w-0) aligns with the broader layout improvements across this PR. Splitting the class strings improves readability while maintaining all existing styles.app/[lang]/(common)/Header/index.tsx (1)
311-319: LGTM! Consistent layout constraint additions.The width constraints added to the header follow the same pattern applied across other layout components in this PR, ensuring consistent overflow prevention and proper width handling throughout the application.
app/[lang]/styles.module.css (1)
6-49: LGTM! Well-designed horizontal scroll utility.The
.horizontalScrollclass is well-implemented with:
- Cross-browser scrollbar support (Firefox via
scrollbar-width, WebKit via pseudo-elements)- Subtle, professional scrollbar styling with transparency and rounded corners
- Mobile-responsive behavior that hides the scrollbar on smaller screens for a cleaner UX
This complements the layout constraint updates and supports the new horizontal UI elements introduced in this PR.
app/[lang]/stats/Container.tsx (1)
23-24: LGTM! Comprehensive width constraint implementation.The explicit width constraints (
flex-1 w-full max-w-full min-w-0) provide robust layout control:
flex-1: Takes available space in flex containerw-full: Ensures 100% width usagemax-w-full: Prevents exceeding container widthmin-w-0: Allows shrinking below content sizeThis follows the same pattern applied consistently throughout the PR's layout improvements.
package.json (1)
47-47: recharts@3.5.1 is valid and has no known direct security vulnerabilities.Version 3.5.1 is the latest official release of recharts and is not marked as deprecated. The official recharts package has no CVE or GHSA entries. Standard practice: run
npm auditto check transitive dependencies (e.g., d3-color for known ReDoS issues), and ensure typo-squat packages like "recharts-smart" are not accidentally installed.locales/en.json (2)
87-96: LGTM!The new localization keys for the leaderboard feature (
all,topUsers,topRanked) and the updated strings (title,noRecentList) are well-structured and consistent with the existing naming conventions.
111-115: LGTM!The new stats-related localization keys (
selectPeriod,periodDescription,resetToDefault,search) are appropriate for the month-picker and search functionality.CLAUDE.md (1)
1-98: LGTM! Excellent addition of project guidelines.This documentation provides clear and actionable guidance for development, covering styling patterns, dark mode support, and component conventions. The examples are practical and align well with Tailwind best practices.
locales/ko.json (2)
96-105: LGTM!The Korean translations for the leaderboard section are consistent with the English counterparts and maintain proper structure.
120-124: LGTM!The Korean translations for the new stats keys are appropriate and match the en.json structure.
app/[lang]/recent-list/GithubUserList.tsx (1)
151-207: LGTM! Well-structured tier filter UI.The tier filter implementation has good UX considerations: proper loading states with disabled buttons and opacity changes, responsive design for mobile, and accessible aria-labels. The use of
useCallbackforhandleTierSelectis appropriate given its use in button handlers.pages/api/top-tier-users.ts (1)
11-61: LGTM! Well-structured API handler.The handler has proper method validation, sequential data fetching with appropriate null checks, and clean data transformation. The tier calculation using
getTierNameis correctly applied.pages/api/users-by-tier.ts (1)
58-61: Off-by-one edge case for max score boundary.When
maxScoreis calculated assortedTiers[tierIndex + 1].score - 1, users exactly at the boundary score of the next tier are correctly excluded. However, for the top tier, the hardcoded100as max might not align if scores can exceed 100.Please verify that scores are guaranteed to be ≤ 100, or adjust the max score logic for the top tier to use a value like
Number.MAX_SAFE_INTEGERor query the actual max score from the database.styles/output.css (1)
1478-1509: This is auto-generated CSS - no action needed.The static analysis warning about duplicate
min-widthproperties appears to be a false positive. These are distinct Tailwind utility classes (.min-w-0and.min-w-fit) with different selectors, not duplicate properties within the same rule. Since this file is auto-generated by Tailwind CSS, manual modifications should be avoided.server/services/githubService.ts (4)
51-66: LGTM! Robust date handling with UTC.The date parsing correctly uses UTC to avoid timezone issues, and the fallback to 12 months ago is well-implemented. Starting from the 1st of the month at noon UTC is a good practice to avoid edge cases.
535-599: Good caching strategy - skipping DB writes for custom date queries.The logic to skip database updates when a custom
startDateis provided is a sensible optimization. This prevents cached default data from being overwritten by date-specific queries, maintaining data integrity for the default view.
811-831: Public API correctly handles errors and returns empty array.The
getMonthlyContributionexport properly logs errors with context and returns an empty array as a safe fallback, allowing callers to handle missing data gracefully.
390-416: I'll help you verify the review comment about thegenerateMonthsfunction. Let me investigate the codebase to understand the context and validate the concerns raised.
<function_calls>
#!/bin/bashFirst, find the file and examine the context around the function
cat -n server/services/githubService.ts | sed -n '385,420p'
</function_calls>
<function_calls>
#!/bin/bashSearch for where generateMonths is called and how startDate is passed
rg "generateMonths" --type ts -B5 -A5
</function_calls>
<function_calls>
#!/bin/bashLook for API endpoints and validation logic related to startDate
rg "startDate" --type ts -B3 -A3 | head -80
</function_calls>
<function_calls>
#!/bin/bashSearch for validation functions or middleware that might validate startDate
rg "(validate|valid|check)" --type ts server/services/githubService.ts -B2 -A2
</function_calls>app/[lang]/stats/[login]/Scouter/StatsDetails/SectionPeople.tsx (1)
7-7: LGTM! Radar chart integration is clean.The StatsRadar component is properly imported and integrated after the scores block, with correct props passed through.
Also applies to: 62-63
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsRadar.tsx (1)
18-27: Good hydration mismatch prevention.The requestAnimationFrame pattern correctly defers rendering until after the client-side hydration completes, preventing React hydration mismatches.
app/[lang]/stats/[login]/Scouter/StatsDetails/index.tsx (2)
23-26: LGTM! Clean endDate propagation to Dooboo section.The endDate prop is correctly destructured and passed only to the SectionDooboo component, which aligns with the monthly contribution feature being specific to the Dooboo stats view.
36-41: Good responsive layout constraints.The addition of
min-w-0enables proper flex shrinking, andoverflow-x-hiddenprevents unwanted horizontal scrolling. These classes work together to ensure the stats details adapt properly to container constraints.app/[lang]/stats/[login]/Scouter/index.tsx (2)
27-35: LGTM! Clean type extension for monthly contributions.The StatsWithMonthly type extends DoobooStatsResponse with optional monthly contributions, and ScouterProps is updated accordingly. The optional nature of both monthlyContributions and endDate allows for backward compatibility.
39-56: Good prop destructuring and propagation.The endDate is cleanly destructured from props and passed through to StatsDetails. The addition of
min-w-0ensures proper flex behavior in constrained layouts.app/[lang]/recent-list/TopTierUsers.tsx (1)
81-83: Good security attributes on external links.The use of
target="_blank"withrel="noopener noreferrer"correctly prevents security vulnerabilities when opening external GitHub profile links.app/[lang]/stats/[login]/page.tsx (3)
46-49: Good use of parallel fetching.Using
Promise.allto fetch stats and monthly contributions concurrently reduces total loading time compared to sequential fetches.
32-43: Date calculation logic is correct.All test cases pass with expected results. The
month - 11calculation properly computes 12 months before the end date, with JavaScript's Date constructor correctly handling the negative month values to wrap into the previous year. No issues found.
17-17: The use offorce-dynamicis appropriate and should not be changed.The page fetches user-specific, frequently-changing GitHub statistics (commits, PRs, reviews, contributions). These are not static data—they update as the user makes contributions. The
getDoobooStatsfunction already implements intelligent caching with a 3-hour background refresh strategy for cached data, andgetMonthlyContributionfetches current data from the GitHub GraphQL API.Using Incremental Static Regeneration (ISR) would be inappropriate here since:
- Data is per-user and changes daily
- Each GitHub user should see their current stats
- ISR caches by URL, which would serve stale data across requests
The
force-dynamicsetting correctly ensures each request fetches fresh data while the underlying Supabase/GitHub layers manage their own caching strategies.app/[lang]/recent-list/page.tsx (2)
56-78: Good responsive layout for horizontal scroll region.The nested structure with
horizontalScrollstyle and responsive breakpoints ensures the TopTierUsers and GreatFrontEnd components display properly across screen sizes. The use ofmin-w-fiton the inner container prevents premature wrapping, and the mobile layout switches to a column layout appropriately.
43-44: Layout alignment changed to items-start.The change from
items-center justify-betweentoitems-startaligns items to the top. Ensure this is the intended visual change, especially for the H1 title alignment relative to the new TopTierUsers component.Verify the visual alignment is correct in the UI, particularly on different screen sizes where the H1 and TopTierUsers might have different heights.
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (2)
35-47:router.refresh()afterrouter.push()may be redundant.Calling
router.refresh()immediately afterrouter.push()can cause unnecessary re-fetching. Next.js App Router'spush()with a new URL typically triggers a navigation that re-fetches server components. The refresh call may be redundant or cause a double data fetch.Verify if the refresh is actually needed for your use case. If the stats data is fetched server-side based on the URL query param,
router.push()alone should suffice:const handleEndDateChange = useCallback( (newDate: string | undefined) => { if (!pathname) return; setIsLoading(true); if (newDate) { router.push(`${pathname}?endDate=${newDate}`); } else { router.push(pathname); } - router.refresh(); }, [pathname, router], );
190-205: LGTM!The component structure cleanly passes
endDatethrough props and properly spreads remaining props to child components. The layout with flex and width constraints looks appropriate.app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (3)
35-41: Good hydration fix using requestAnimationFrame.Using
requestAnimationFrameto defersetIsClient(true)avoids React 19's strict mode warnings about synchronous state updates in effects. This is a clean pattern for client-only rendering guards.
43-48: Acceptable use ofanyfor Recharts event handlers.The
eslint-disablecomments for@typescript-eslint/no-explicit-anyare reasonable here since Recharts' event handler types are not well-typed. The code properly validatesdata?.dataKeybefore use, which is defensive.Also applies to: 54-67
137-138: Chart width could grow unbounded with many months.The width calculation
Math.max(640, chartData.length * 60)grows linearly with data length. For 12+ months of data, this works fine, but verify the expected data range. The horizontal scroll container handles overflow gracefully.app/[lang]/stats/[login]/MonthPicker.tsx (1)
151-162: I'll help you verify whether thespinkeyframe animation is properly defined in this codebase. Let me search for its definition and understand the CSS setup.
<function_calls>
#!/bin/bashSearch for @Keyframes spin definition in CSS/SCSS files
echo "=== Searching for @Keyframes spin ==="
rg -n '@Keyframes\s+spin' --type css --type scssecho ""
echo "=== Searching for spin animation references ==="
rg -n 'spin' --type css --type scss | head -20echo ""
echo "=== Checking for Tailwind config ==="
fd -n "tailwind|tailwind.config" -e js -e ts -e json
</function_calls>
<function_calls>
#!/bin/bashCheck the MonthPicker.tsx file and surrounding files
echo "=== Checking MonthPicker.tsx imports ==="
fd -p "MonthPicker.tsx" | xargs head -30echo ""
echo "=== Looking for global stylesheets ==="
fd -e css -e scss | grep -E "globals|global|styles|app" | head -20echo ""
echo "=== Checking for CSS-in-JS or styled components ==="
rg -n "styled|css`|keyframes" --type tsx --type ts | grep -i spin | head -10
</function_calls>
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (6)
app/[lang]/leaderboard/GithubUserList.tsx (2)
60-60: Hardcoded API endpoint should be centralized.This duplicates a previous review comment. The API endpoint URL should be defined in a centralized constants file for maintainability.
229-232: Use HTTPS and add security parameters for window.open.This duplicates a previous review comment. The GitHub URL should use HTTPS instead of HTTP. Additionally, consider adding security parameters to prevent tabnabbing attacks.
Apply this diff:
onClickRow={(user) => { const login = user.login; - window.open('http://github.com/' + login); + window.open('https://github.com/' + login, '_blank', 'noopener,noreferrer'); }}app/[lang]/leaderboard/TopTierUsers.tsx (1)
25-25: API endpoint URL is hardcoded.The endpoint
/api/top-tier-usersis hardcoded, which makes it harder to maintain and update. Consider extracting API routes to a shared constants file.app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
43-47: Replaceanywith typed event payload.Past review already flagged the weak type safety from using
any. Both handlers receive legend event payloads from recharts with a known shape.Apply this diff to type both handlers:
- const handleLegendMouseEnter = useCallback((data: any) => { + const handleLegendMouseEnter = useCallback((data: { dataKey?: string }) => { if (data?.dataKey) { setHoveredLine(String(data.dataKey)); } }, []); const handleLegendMouseLeave = useCallback(() => { setHoveredLine(null); }, []); - const handleLegendClick = useCallback((data: any) => { + const handleLegendClick = useCallback((data: { dataKey?: string }) => { if (data?.dataKey) { const key = data.dataKey as LineKey; setActiveLines((prev) => {Also applies to: 53-65
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (1)
30-35: Fix ESLint error blocking the pipeline.Past review flagged this synchronous
setStatecall within an effect, and the pipeline is failing. Addingeslint-disablesuppresses the warning but doesn't fix the architectural issue.Apply the solution from the past review — remove this effect and reset loading when data arrives:
- // Reset loading when endDate changes (data loaded) - useEffect(() => { - // Safe reset after data refresh - // eslint-disable-next-line react-hooks/set-state-in-effect - setIsLoading(false); - }, [endDate, stats]);Then, derive
isLoadingfrom props or reset it in the parent component whenstatsupdates. Alternatively, implement the ref-based approach suggested in the past review to track expectedendDate.app/[lang]/stats/[login]/SearchTextInput.tsx (1)
71-81: Spinner/button UX is solid, but replace inline styles with Tailwind (or a shared Spinner) and add an accessible label.
- The button/disabled logic and inline spinner feedback are good, but the spinner is implemented via inline
style={...}; project guidelines prefer Tailwind utilities or a shared component. You could match the prior suggestion and swap to something like:- {isLoading ? ( - <div - style={{ - width: '20px', - height: '20px', - border: '2px solid #BEBEBE', - borderTopColor: '#4190EB', - borderRadius: '50%', - animation: 'spin 1s linear infinite', - }} - /> - ) : ( - <SearchIcon size={20} className="text-basic" /> - )} + {isLoading ? ( + <div + className="h-5 w-5 animate-spin rounded-full border-2 border-gray-300 border-t-brand dark:border-gray-600 dark:border-t-brand" + /> + ) : ( + <SearchIcon size={20} className="text-basic" /> + )}
- For accessibility, consider giving the submit button an explicit label so icon-only content is announced correctly, e.g.
aria-label={t.search}ortitle={t.search}.Also applies to: 94-119
🧹 Nitpick comments (5)
app/[lang]/leaderboard/GithubUserList.tsx (4)
44-48: Cursor initialization is safe but consider adding a comment.The nested optional chaining protects against undefined access, but the logic is non-obvious. A brief comment explaining the cursor initialization would improve maintainability.
66-67: Provide user-facing error feedback.Currently, errors are only logged to the console. Consider displaying an error message or toast notification to inform users when tier data fails to load.
103-103: Remove redundant height class.The className contains both
h-[37px]andh-[50px]. The latter overrides the former, makingh-[37px]redundant.Apply this diff:
- cellClassName: 'h-[37px] text-start w-3/12 h-[50px] py-[8px]', + cellClassName: 'text-start w-3/12 h-[50px] py-[8px]',
114-114: Remove redundant height class.Same issue as Line 103 -
h-[37px]is overridden byh-[50px].Apply this diff:
- cellClassName: 'h-[37px] text-start w-3/12 h-[50px] py-[8px]', + cellClassName: 'text-start w-3/12 h-[50px] py-[8px]',app/[lang]/leaderboard/TopTierUsers.tsx (1)
22-38: Add cleanup with AbortController.The
useEffectshould include anAbortControllerto cancel the fetch if the component unmounts before the request completes, preventing potential memory leaks and state updates on unmounted components.Apply this diff:
useEffect(() => { + const controller = new AbortController(); + const fetchTopUsers = async () => { try { - const response = await fetch('/api/top-tier-users'); + const response = await fetch('/api/top-tier-users', { + signal: controller.signal, + }); const data = await response.json(); if (data.users) { setTopTierUsers(data.users); } } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + return; + } console.error('Failed to fetch top tier users:', error); } finally { setIsLoading(false); } }; fetchTopUsers(); + + return () => { + controller.abort(); + }; }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/[lang]/(common)/Header/index.tsx(6 hunks)app/[lang]/leaderboard/GithubUserList.tsx(1 hunks)app/[lang]/leaderboard/TopTierUsers.tsx(1 hunks)app/[lang]/leaderboard/page.tsx(3 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx(5 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx(1 hunks)app/[lang]/stats/[login]/SearchTextInput.tsx(4 hunks)locales/en.json(4 hunks)locales/ko.json(3 hunks)styles/output.css(38 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/[lang]/stats/[login]/SearchTextInput.tsx (1)
src/hooks/useSearchHistory.ts (1)
useSearchHistory(26-65)
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
server/services/githubService.ts (1)
MonthlyContribution(365-371)
app/[lang]/leaderboard/TopTierUsers.tsx (3)
src/fetches/recentList.ts (1)
UserListItem(5-12)src/utils/functions.ts (1)
getTierSvg(99-115)app/[lang]/leaderboard/TierRowItem.tsx (1)
Tier(6-14)
app/[lang]/leaderboard/page.tsx (2)
app/[lang]/leaderboard/TopTierUsers.tsx (1)
TopTierUsers(18-131)app/[lang]/(common)/GreatFrontEnd.tsx (1)
GreatFrontEnd(16-43)
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (3)
app/[lang]/stats/[login]/Scouter/index.tsx (2)
statNames(18-25)StatsWithMonthly(27-29)app/[lang]/stats/[login]/MonthPicker.tsx (1)
MonthPicker(29-265)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
StatsChart(24-212)
🪛 Biome (2.1.2)
styles/output.css
[error] 1508-1508: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
min-width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (18)
locales/ko.json (2)
96-105: Korean translations for leaderboard and tier filtering look good.The new keys (
all,topUsers,topRanked) and updated labels properly reflect the leaderboard feature and tier-filtering capabilities. Translations maintain consistency with the English locale structure and align with the PR's feature set.
106-124: Stats section translations support monthly tracking feature.The new keys (
selectPeriod,periodDescription,resetToDefault,search,score) are appropriately translated and align with the monthly contribution visualization goals. The updatedsearchUserHintis clear and contextually accurate.locales/en.json (3)
87-95: Leaderboard and tier filter labels are clear and consistent.The rebranding from "Recent List" to "Leaderboard" (line 88) and the new tier-filter keys (
all,topUsers,topRanked) effectively communicate the filtering capabilities. The updated "No users found" message (line 90) is more accurate than the previous "No recent list."
111-111: Inconsistent capitalization forstats.score.In the recentList section (line 92),
"score"is capitalized as"Score", but in the stats section (line 111), it's lowercase"score". This inconsistency may cause unexpected formatting if both keys are used in similar UI contexts (e.g., column headers, badges).Verify whether the lowercase form is intentional for stats display or should be capitalized to match recentList.
Consider updating line 111 to maintain consistency:
- "score": "score", + "score": "Score",
97-115: Stats section translations properly support monthly tracking and period selection.The new keys (
selectPeriod,periodDescription,resetToDefault,search) are well-phrased and clearly communicate the UI interactions for monthly stats navigation. The updatedsearchUserHint(line 99) provides better context than the previous message.styles/output.css (1)
1506-1509: Static analysis flag is a false positive — vendor prefixes are intentional.Line 1508 flags a duplicate
min-widthproperty, but this is standard CSS practice: the-moz-vendor prefix and the standard property coexist for cross-browser fallback support. No action needed.app/[lang]/leaderboard/GithubUserList.tsx (4)
1-31: LGTM! Clean imports and tier ordering.The component structure and tier hierarchy are well-defined.
73-74: LGTM! Clean data source switching.The conditional logic correctly selects between tier-filtered and unfiltered data.
151-207: LGTM! Well-structured tier filter UI.The tier selection buttons have appropriate loading states, responsive styling, and accessibility attributes.
209-242: Well-designed data table container.The table wrapper has appropriate styling, loading states, and conditional scroll handling. The scroll handler is correctly disabled when a tier is selected.
app/[lang]/leaderboard/page.tsx (2)
43-44: Layout restructuring looks good.The shift from
justify-betweentogap-4withitems-startalignment properly supports the new horizontal scroll container while maintaining responsive behavior for mobile viewports.
56-78: Horizontal scroll container structure is functional.The three-level nesting (scroll container → nowrap flex → components) correctly implements horizontal scrolling with fixed-width children. The responsive visibility controls are appropriate for the layout constraints.
app/[lang]/(common)/Header/index.tsx (3)
49-54: Path matching logic is functional.The
normalizePathandisActivePathfunctions correctly handle trailing slashes and provide both exact and prefix matching. ThestartsWithapproach works well for the current navigation structure (/stats/${login},/leaderboard).
265-265: Navigation path updates are correct.The route rename from
/recent-listto/leaderboardaligns with the PR objectives, and removing the trailing slash from/stats/ensures consistency with the newnormalizePathutility.Also applies to: 271-271, 275-275
325-325: Layout constraint classes are appropriate.Adding
w-full min-w-0to the header andmin-w-0to the inner container correctly prevents flex overflow issues and enables proper text truncation behavior.Also applies to: 333-333
app/[lang]/leaderboard/TopTierUsers.tsx (3)
1-16: Imports and type definitions are clean.The component correctly imports necessary dependencies and defines a clear props interface.
40-62: Loading skeleton provides good UX.The shimmer effect with consistent styling and dark mode support creates a polished loading experience.
68-130: Main render logic is well-implemented.The horizontal scroll container, user tiles, and interactive states are correctly structured. The component properly handles Next.js Image optimization, external links with security attributes, focus states, and responsive design.
| cursor, | ||
| }); | ||
|
|
||
| const filteredUsers = users.filter((el) => !data.includes(el)); |
There was a problem hiding this comment.
Critical: Object comparison by reference won't deduplicate users.
The data.includes(el) check compares objects by reference, not by value. This means duplicate users (with the same data but different object instances) will not be filtered out, leading to duplicate entries in the list.
Apply this diff to compare by unique identifier (login):
- const filteredUsers = users.filter((el) => !data.includes(el));
+ const filteredUsers = users.filter((el) => !data.some(existing => existing.login === el.login));📝 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 filteredUsers = users.filter((el) => !data.includes(el)); | |
| const filteredUsers = users.filter((el) => !data.some(existing => existing.login === el.login)); |
🤖 Prompt for AI Agents
In app/[lang]/leaderboard/GithubUserList.tsx around line 144, the filter uses
data.includes(el) which compares object references and fails to deduplicate
users with identical content; change the logic to compare by unique identifier
(login) — build a Set of existing logins from data (e.g. new Set(data.map(d =>
d.login))) and filter users by checking if the user's login is in that Set (or
use data.some(d => d.login === el.login)) so duplicates are removed by value
rather than reference.
| const filteredUsers = users.filter((el) => !data.includes(el)); | ||
|
|
||
| setData([...data, ...filteredUsers]); | ||
| setCursor(new Date(filteredUsers?.[filteredUsers.length - 1]?.createdAt)); |
There was a problem hiding this comment.
Guard against empty filteredUsers array.
If filteredUsers is empty after filtering, accessing filteredUsers[filteredUsers.length - 1] will be undefined, resulting in new Date(undefined) which creates an invalid date and breaks pagination.
Apply this diff to add a guard:
- setData([...data, ...filteredUsers]);
- setCursor(new Date(filteredUsers?.[filteredUsers.length - 1]?.createdAt));
+ if (filteredUsers.length > 0) {
+ setData([...data, ...filteredUsers]);
+ setCursor(new Date(filteredUsers[filteredUsers.length - 1].createdAt));
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/[lang]/leaderboard/GithubUserList.tsx around line 147, the code
unconditionally does setCursor(new Date(filteredUsers?.[filteredUsers.length -
1]?.createdAt)); which will call new Date(undefined) when filteredUsers is
empty; change it to first check that filteredUsers exists and has length > 0 and
only then call setCursor with the last item's createdAt, otherwise setCursor to
null/undefined (or skip the call) to avoid creating an invalid Date and breaking
pagination.
| const handleEndDateChange = useCallback( | ||
| (newDate: string | undefined) => { | ||
| if (!pathname) return; | ||
| setIsLoading(true); | ||
| if (newDate) { | ||
| router.push(`${pathname}?endDate=${newDate}`); | ||
| } else { | ||
| router.push(pathname); | ||
| } | ||
| router.refresh(); | ||
| }, | ||
| [pathname, router], | ||
| ); |
There was a problem hiding this comment.
Loading state management is fragile and may cause stuck spinners.
The pattern of setting isLoading(true) here and resetting it in a useEffect dependent on endDate/stats creates a race condition. If the parent component doesn't propagate the new endDate back down (e.g., fetch fails, navigation interrupted), isLoading remains true indefinitely, leaving the UI in a perpetual loading state.
Lift the loading state to the parent component that fetches the data, or use a data-fetching library (like React Query) that tracks loading/error/success states. The child component should receive isLoading as a prop from the source of truth.
Alternatively, if you must manage it here, track the expected endDate in a ref and only reset loading when the incoming stats actually corresponds to that endDate:
const expectedEndDateRef = useRef<string | undefined>(endDate);
const handleEndDateChange = useCallback(
(newDate: string | undefined) => {
if (!pathname) return;
expectedEndDateRef.current = newDate;
setIsLoading(true);
// ... push & refresh
},
[pathname, router],
);
useEffect(() => {
if (expectedEndDateRef.current === endDate) {
setIsLoading(false);
}
}, [endDate]);🤖 Prompt for AI Agents
In app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx around lines
37 to 49, the component sets isLoading(true) locally when changing endDate which
can lead to a stuck spinner if the parent does not propagate the updated data;
either lift isLoading to the parent (pass isLoading as a prop and let the parent
set/clear it based on actual fetch success/failure or use a data-fetching lib
like React Query to manage loading/error states), or if you must keep local
state, track the expected endDate in a ref before router.push and only clear
isLoading when incoming props/stats match that expected endDate (update
expectedEndDateRef before navigation and compare it in an effect that clears
loading), and remove any reliance on a generic endDate/stats effect that could
leave loading true on failed/aborted navigations.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/plugins/stats/gold.ts (1)
166-287: Fix GOLD language counts and collaborated‑repo detail sourceThere are a few logic issues that skew the GOLD stats:
langsMyCountis computed fromuniqueContribLangsinstead ofuniqueMyLangs, so “My Languages” count and the GOLD score use contributed‑repo language data twice.- The
MY_LANGUAGESstat’stotalCountuseslangsContribCountinstead oflangsMyCount.- The
STARS_IN_COLLABORATED_REPOSdetails are built fromcontribRepoPRsinstead ofcollaboratedRepos, so the list does not match thecollaboratedReposStarCountmetric.A minimal fix:
- const langsContribCount = uniqueContribLangs.filter( - (el) => el.count > 3, - ).length; - const langsMyCount = uniqueContribLangs.filter((el) => el.count > 3).length; + const langsContribCount = uniqueContribLangs.filter( + (el) => el.count > 3, + ).length; + const langsMyCount = uniqueMyLangs.filter((el) => el.count > 3).length; @@ { key: 'STARS_IN_COLLABORATED_REPOS', @@ - totalCount: collaboratedReposStarCount, - details: JSON.stringify( - contribRepoPRs + totalCount: collaboratedReposStarCount, + details: JSON.stringify( + collaboratedRepos .filter((el) => !!el) .map((el) => { @@ { key: 'MY_LANGUAGES', @@ - totalCount: langsContribCount, - details: JSON.stringify(uniqueMyLangs), + totalCount: langsMyCount, + details: JSON.stringify(uniqueMyLangs), },This aligns counts and detail lists with the intended datasets and ensures the GOLD score reflects the correct language mix.
♻️ Duplicate comments (4)
app/[lang]/leaderboard/GithubUserList.tsx (2)
127-149: Fix infinite scroll deduplication and cursor handlingThe current infinite scroll logic still has two issues:
users.filter((el) => !data.includes(el))compares object references, so duplicate users with the sameloginwill not be removed and can appear multiple times.- When
filteredUsersis empty,filteredUsers?.[filteredUsers.length - 1]?.createdAtisundefined, sonew Date(undefined)yields an invalidDate. That invalid cursor will still be truthy, so subsequent fetches may be called with an invalid cursor value.Consider instead deduping by a stable key (e.g.,
login) and guarding against the empty case:const {users} = await fetchRecentList({ pluginId: 'dooboo-github', take: 20, cursor, }); - const filteredUsers = users.filter((el) => !data.includes(el)); - - setData([...data, ...filteredUsers]); - setCursor(new Date(filteredUsers?.[filteredUsers.length - 1]?.createdAt)); + const existingLogins = new Set(data.map((u) => u.login)); + const filteredUsers = users.filter( + (u) => !existingLogins.has(u.login), + ); + + if (filteredUsers.length === 0) { + return; + } + + setData([...data, ...filteredUsers]); + setCursor( + new Date(filteredUsers[filteredUsers.length - 1].createdAt), + );This avoids duplicates by value and ensures the cursor is only advanced when new items exist.
51-72: Still missing response status check for tier fetch
handleTierSelectcallsresponse.json()without verifyingresponse.ok, so 4xx/5xx responses (or HTML error pages) could throw or yield unexpected shapes.Adding a simple status check before
response.json()would make this more robust:setIsLoadingTier(true); try { const response = await fetch(`${API_USERS_BY_TIER}?tier=${tier}`); + if (!response.ok) { + throw new Error(`Failed to fetch tier users: ${response.status}`); + } const result = await response.json(); if (result.users) { setTierData(result.users); }app/[lang]/leaderboard/TopTierUsers.tsx (1)
65-67: Improve empty state handling.Returning an empty
<div />is neither semantic nor accessible. Consider returningnullinstead.app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
82-100: Add validation for month format.The function assumes
monthis always in validYYYY-MMformat. If the backend returns malformed data,parseInt(monthNum, 10) - 1could produce an invalid index (e.g., -1, NaN, or 12), returningundefined.
🧹 Nitpick comments (8)
server/plugins/stats/fire.ts (1)
16-45: FIRE repo mapping null‑safety is improved; consider also guarding null edges for consistencyThe new
ownerextraction +return null+ type‑guard.filterpattern ensures you don’t generate brokenowner/nameURLs or objects with missing owners. That’s a nice robustness win.For consistency with other plugin mappers (e.g., gold/trophies using
if (!el) return null;before accessingel.node), you might also add an earlyif (!el) return null;here, though GraphQL edges are typically non‑null.server/services/githubService.ts (3)
275-327: Monthly contributions helper is sound; consider surfacing partial failuresThe monthly contributions query correctly bounds each month with UTC
from/tovalues and returns zero counts when the GitHub request fails, which is safe.If you need to debug per‑month failures later, you might want to optionally log or tag which month failed (even at a lower log level) rather than silently returning zeros in the
catchpath ofgetMonthlyContributions. For now, the behavior is acceptable.
454-599: Avoid overwriting cached stats for customstartDate– behavior looks correctThe
if (!startDate)guard around all Supabase writes inupsertGithubStatsensures that ad‑hoc, date‑bounded stats requests don’t mutate the default cached dataset. PassingstartDateonly intogetGithubUserwhile keeping DB writes behind the guard is a clean separation of concerns.No functional issues spotted here.
811-832:getMonthlyContributionwrapper is fine; consider reusing validationThe public API wrapper lowercases
login, delegates togetMonthlyContributions, and returns an empty array on error with useful logging — that’s a reasonable contract.If you adopt stronger
startDatevalidation ingetGithubUser, you might want to reuse the same validation (or a shared helper) here to ensure both entry points interpretstartDateidentically.pages/api/top-tier-users.ts (1)
1-61: Top-tier users API is well-structured; consider sharing the tier typeThe handler correctly:
- Restricts to GET, 404s when the plugin is missing.
- Pulls the top 5
user_pluginsby score fordooboo-github.- Derives
tierNameviagetTierNameand filters out rows withgithub_id === null.- Adds
Cache-Control: s-maxage=3600, stale-while-revalidate.One small clean‑up: you could reuse the same
TierDeftype used inpages/api/users-by-tier.tsinstead of re‑declaring{tier: string; score: number}here, to keep tier JSON typing in one place.app/[lang]/leaderboard/TopTierUsers.tsx (1)
104-110: Unsafe type assertion for tierName.The
user.tierName as Tiercast assumes the API always returns valid tier names. If the API returns an unexpected value,getTierSvgwill default to Iron tier silently, which may hide data issues.Consider adding runtime validation or using a type guard:
const isValidTier = (tier: string): tier is Tier => ['Iron', 'Bronze', 'Silver', 'Gold', 'Platinum', 'Diamond', 'Master', 'Challenger'].includes(tier); // Then in render: src={getTierSvg(isValidTier(user.tierName) ? user.tierName : 'Iron')}app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
136-140: Consider ResponsiveContainer for better chart responsiveness.The current implementation uses a fixed width calculation. Recharts provides
ResponsiveContainerwhich handles resizing automatically and may provide a smoother experience.+import { ResponsiveContainer } from 'recharts'; -<LineChart - width={Math.max(640, chartData.length * 60)} - height={300} - ... -> +<ResponsiveContainer width="100%" height={300}> + <LineChart data={chartData} margin={{top: 10, right: 30, left: 10, bottom: 20}}> + ... + </LineChart> +</ResponsiveContainer>app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (1)
50-54: Consider using router.replace instead of router.push for filter changes.Using
router.pushadds entries to the browser history for each date change. For filter/query param changes,router.replaceis often preferred to avoid polluting the history stack.if (newDate) { - router.push(`${pathname}?endDate=${newDate}`); + router.replace(`${pathname}?endDate=${newDate}`); } else { - router.push(pathname); + router.replace(pathname); } router.refresh();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/[lang]/leaderboard/GithubUserList.tsx(1 hunks)app/[lang]/leaderboard/TopTierUsers.tsx(1 hunks)app/[lang]/leaderboard/apiRoutes.ts(1 hunks)app/[lang]/stats/[login]/MonthPicker.tsx(1 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx(5 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx(1 hunks)app/[lang]/stats/[login]/SearchTextInput.tsx(4 hunks)pages/api/top-tier-users.ts(1 hunks)server/plugins/stats/fire.ts(1 hunks)server/plugins/stats/gold.ts(1 hunks)server/services/githubService.ts(13 hunks)styles/output.css(42 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/[lang]/stats/[login]/MonthPicker.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
app/[lang]/leaderboard/TopTierUsers.tsx (4)
src/fetches/recentList.ts (1)
UserListItem(5-12)app/[lang]/leaderboard/apiRoutes.ts (1)
API_TOP_TIER_USERS(2-2)src/utils/functions.ts (1)
getTierSvg(99-115)app/[lang]/leaderboard/TierRowItem.tsx (1)
Tier(6-14)
app/[lang]/stats/[login]/SearchTextInput.tsx (1)
src/hooks/useSearchHistory.ts (1)
useSearchHistory(26-65)
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
server/services/githubService.ts (1)
MonthlyContribution(365-371)
pages/api/top-tier-users.ts (4)
src/utils/functions.ts (1)
PluginUser(43-58)pages/api/users-by-tier.ts (1)
handler(13-89)server/supabaseClient.ts (1)
getSupabaseClient(9-17)src/types/types.ts (1)
UserPluginRow(10-10)
server/services/githubService.ts (4)
server/plugins/types/UserGraph.ts (1)
UserGraph(1-31)server/plugins/index.ts (1)
getGithubStatus(66-93)server/plugins/topLanguages.ts (1)
getTopLanguages(9-72)server/plugins/trophies.ts (1)
getTrophies(140-282)
🪛 Biome (2.1.2)
styles/output.css
[error] 1508-1508: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
min-width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (10)
app/[lang]/leaderboard/apiRoutes.ts (1)
1-2: Centralized leaderboard API route constants look goodDefining
API_USERS_BY_TIERandAPI_TOP_TIER_USERShere removes hard‑coded paths in components and keeps routes consistent.styles/output.css (1)
1506-1509: Duplicatemin-widthin.min-w-fitis intentional, not a bugThe two
min-widthdeclarations in.min-w-fit(-moz-fit-contentthenfit-content) are the usual vendor‑prefixed + standard pattern generated by Tailwind. Functionally this is correct; the second line just overrides the first where supported.If Biome is flagging this as
noDuplicateProperties, it’s safe to suppress that rule for generated CSS rather than changing the output.server/services/githubService.ts (2)
390-452: Monthly window generation & batching look reasonable
generateMonthsandgetMonthlyContributionscorrectly:
- Start from the given
startDatemonth (UTC) and advance up to 12 months.- Exclude future months relative to current UTC year/month.
- Limit concurrency to batches of 3 GitHub GraphQL calls.
That’s a good balance between correctness and rate‑limit friendliness. No changes needed here.
631-799: Cache‑skip semantics forgetDoobooStatswithstartDateare consistent
getDoobooStatsnow:
- Skips the cached
user_plugins+statsfast‑path when a customstartDateis provided.- Still uses the cached path (with background
upsertGithubStats) when!startDateand cached stats exist.- Propagates
startDatethrough toupsertGithubStatsin the miss path.This correctly prevents custom date queries from polluting the default cache while keeping the existing ISR‑like behavior intact. Looks good.
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (2)
36-42: Good use of requestAnimationFrame for hydration safety.The deferred
setIsClient(true)viarequestAnimationFramecorrectly avoids React 19 hydration warnings. The cleanup function properly cancels the frame on unmount.
44-66: Legend interaction handlers are well-typed.Using
LegendPayloadfrom recharts provides proper type safety. The click behavior that resets to all lines when clicking the last active line is a good UX pattern.app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (3)
31-42: Improved loading state management with ref pattern.The
pendingEndDateRefapproach addresses the previous ESLint warning and race condition concerns. TherequestAnimationFramewrapper defers the state update appropriately.However, the condition
pendingEndDateRef.current === undefinedon line 35 will reset loading on initial mount even if no navigation occurred. This may be intentional for the initial load, but verify this is the desired behavior.
139-149: StatsChart and MonthPicker integration looks correct.The components receive appropriate props:
MonthPickergets the translation object, current value, change handler, and loading state.StatsChartreceives the monthly contributions data and loading state for visual feedback.
194-198: Type definition updated correctly.The
SectionPropstype now properly includesendDate?: stringand usesStatsWithMonthlywhich includes the monthly contributions data needed byStatsChart.app/[lang]/leaderboard/TopTierUsers.tsx (1)
94-103: GitHub avatar domain is already properly configured.The
next.config.jsfile includes the correctimages.remotePatternsconfiguration foravatars.githubusercontent.comwith thehttpsprotocol and/u/**pathname pattern, which matches GitHub avatar URLs. No configuration changes are needed.
| useEffect(() => { | ||
| const fetchTopUsers = async () => { | ||
| try { | ||
| const response = await fetch(API_TOP_TIER_USERS); | ||
| const data = await response.json(); | ||
| if (data.users) { | ||
| setTopTierUsers(data.users); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to fetch top tier users:', error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchTopUsers(); | ||
| }, []); |
There was a problem hiding this comment.
Missing response status check before parsing JSON.
The fetch doesn't verify response.ok before calling response.json(). If the API returns a non-2xx status (e.g., 404, 500), parsing may fail or return unexpected data.
Apply this diff to add proper response handling:
useEffect(() => {
const fetchTopUsers = async () => {
try {
const response = await fetch(API_TOP_TIER_USERS);
+ if (!response.ok) {
+ throw new Error(`HTTP error: ${response.status}`);
+ }
const data = await response.json();
if (data.users) {
setTopTierUsers(data.users);
}
} catch (error) {
console.error('Failed to fetch top tier users:', error);
} finally {
setIsLoading(false);
}
};
fetchTopUsers();
}, []);📝 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.
| useEffect(() => { | |
| const fetchTopUsers = async () => { | |
| try { | |
| const response = await fetch(API_TOP_TIER_USERS); | |
| const data = await response.json(); | |
| if (data.users) { | |
| setTopTierUsers(data.users); | |
| } | |
| } catch (error) { | |
| console.error('Failed to fetch top tier users:', error); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; | |
| fetchTopUsers(); | |
| }, []); | |
| useEffect(() => { | |
| const fetchTopUsers = async () => { | |
| try { | |
| const response = await fetch(API_TOP_TIER_USERS); | |
| if (!response.ok) { | |
| throw new Error(`HTTP error: ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| if (data.users) { | |
| setTopTierUsers(data.users); | |
| } | |
| } catch (error) { | |
| console.error('Failed to fetch top tier users:', error); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; | |
| fetchTopUsers(); | |
| }, []); |
🤖 Prompt for AI Agents
In app/[lang]/leaderboard/TopTierUsers.tsx around lines 23 to 39, the fetch
handler parses response.json() without checking response.ok; update
fetchTopUsers to first check response.ok after awaiting fetch, and if not ok
read response.text() (or response.json() guarded in try) and throw or log a
descriptive error including status and body so we don't attempt to use
unexpected data; only call setTopTierUsers with data.users when the response is
ok and parsed successfully, keep the existing catch/finally to handle and
surface errors and ensure setIsLoading(false) still runs.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
app/[lang]/leaderboards/TopTierUsers.tsx (1)
68-70: Empty state could returnnullfor clarity.Returning an empty fragment
<></>works, but returningnullis more explicit and semantically clearer for "render nothing" scenarios.if (topTierUsers.length === 0) { - return <></>; + return null; }app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (2)
82-104: Month format validation looks good.The validation for invalid month indices (NaN, out of range) with a fallback to the original string addresses the previous review concern.
44-48: Type safety improved for legend handlers.Using
LegendPayloadinstead ofanyaddresses the previous type safety concern from the past review.app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (2)
31-42: Loading state management improved with ref pattern.The ref-based approach addresses the previous ESLint warning and race condition concerns. Tracking
pendingEndDateRefand only clearing loading when the incomingendDatematches (or on initial mount) is a sound solution.
44-56: Navigation handler looks correct.The callback properly sets the pending ref before navigation and triggers loading state. The dependency array correctly includes
pathnameandrouter.
🧹 Nitpick comments (6)
server/services/githubService.ts (1)
402-428: Consider validatingstartDateingenerateMonths.The
generateMonthsfunction assumesstartDateis a validYYYY-MMstring. If called with a malformed input,yearandmonthwill beNaN, leading to unexpected behavior in the loop. AlthoughgetGithubUservalidates itsstartDate,generateMonthsis called through a different path viagetMonthlyContributions.Consider adding validation consistent with
getGithubUser:const generateMonths = (startDate: string): string[] => { const months: string[] = []; const [year, month] = startDate.split('-').map(Number); + + // Validate startDate format + if (!Number.isFinite(year) || !Number.isFinite(month) || month < 1 || month > 12) { + // Fallback to current month minus 12 + const now = new Date(); + const fallbackYear = now.getUTCFullYear(); + const fallbackMonth = now.getUTCMonth(); // 0-indexed, so this is 12 months ago + return generateMonths(`${fallbackYear}-${String(fallbackMonth + 1).padStart(2, '0')}`); + } // Get current year and month in UTC const now = new Date();app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
119-128: Consider edge case when all lines are inactive.If
activeLinesbecomes empty (which shouldn't happen with current logic, but defensively),maxValuewould be-InfinityfromMath.max()on an empty spread. The|| 10fallback onyAxisMaxpartially handles this, butMath.ceil(-Infinity * 1.1)returns-Infinity, not a falsy value.const maxValue = Math.max( + 0, ...chartData.flatMap((d) => { const values: number[] = []; if (activeLines.has('commits')) values.push(d.commits); if (activeLines.has('pullRequests')) values.push(d.pullRequests); if (activeLines.has('reviews')) values.push(d.reviews); return values.length > 0 ? values : [0]; }), );app/[lang]/leaderboards/GithubUserList.tsx (2)
110-110: Unsafe type cast for tierName.Casting
tierName as Tierwithout validation could cause issues if the API returns an unexpected tier value. Consider adding a type guard or fallback.- cell: ({tierName}) => <TierRowItem tier={tierName as Tier} />, + cell: ({tierName}) => ( + <TierRowItem tier={TIER_ORDER.includes(tierName as Tier) ? (tierName as Tier) : 'Iron'} /> + ),
234-237: Consider using Next.js Link for navigation.Opening GitHub profiles via
window.openworks, but using a standard anchor tag or Next.jsLinkwould be more accessible and allow users to right-click for context menu options.app/[lang]/stats/[login]/SearchTextInput.tsx (2)
42-50: Consider removingrequestAnimationFramefor simpler loading state reset.The
requestAnimationFramewrapper defers the check by one frame but doesn't provide a clear benefit here. The effect already runs after the component renders with the newinitialValue, so the check can be synchronous:- useEffect(() => { - const frame = requestAnimationFrame(() => { - if (pendingLoginRef.current && initialValue === pendingLoginRef.current) { - pendingLoginRef.current = null; - setIsLoading(false); - } - }); - return () => cancelAnimationFrame(frame); - }, [initialValue]); + useEffect(() => { + if (pendingLoginRef.current && initialValue === pendingLoginRef.current) { + pendingLoginRef.current = null; + setIsLoading(false); + } + }, [initialValue]);This reduces complexity without changing behavior.
52-61: Consider adding a timeout to reset loading state.If navigation fails or is interrupted,
isLoadingcould remaintrueindefinitely, leaving the button disabled. A timeout provides a safety net:const navigateTo = (loginValue: string) => { if (loginValue === initialValue) { return; } pendingLoginRef.current = loginValue; setIsLoading(true); router.push(`/${lang}/stats/${loginValue}`); + + // Safety timeout in case navigation fails + setTimeout(() => { + if (pendingLoginRef.current === loginValue) { + pendingLoginRef.current = null; + setIsLoading(false); + } + }, 5000); };While Next.js navigation rarely fails, this ensures better UX resilience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CLAUDE.md(1 hunks)app/[lang]/(common)/Header/index.tsx(5 hunks)app/[lang]/leaderboards/GithubUserList.tsx(1 hunks)app/[lang]/leaderboards/TopTierUsers.tsx(1 hunks)app/[lang]/leaderboards/apiRoutes.ts(1 hunks)app/[lang]/leaderboards/page.tsx(3 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx(5 hunks)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx(1 hunks)app/[lang]/stats/[login]/SearchTextInput.tsx(4 hunks)locales/en.json(4 hunks)locales/ko.json(3 hunks)server/services/githubService.ts(13 hunks)styles/output.css(41 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/[lang]/(common)/Header/index.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
app/[lang]/leaderboards/page.tsx (3)
src/localization.ts (1)
getTranslates(17-23)app/[lang]/leaderboards/TopTierUsers.tsx (1)
TopTierUsers(19-135)app/[lang]/leaderboards/GithubUserList.tsx (1)
GithubUserList(39-249)
app/[lang]/stats/[login]/SearchTextInput.tsx (1)
src/hooks/useSearchHistory.ts (1)
useSearchHistory(26-65)
app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (3)
app/[lang]/stats/[login]/Scouter/index.tsx (2)
statNames(18-25)StatsWithMonthly(27-29)app/[lang]/stats/[login]/MonthPicker.tsx (1)
MonthPicker(29-226)app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
StatsChart(25-217)
app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (1)
server/services/githubService.ts (1)
MonthlyContribution(377-383)
server/services/githubService.ts (2)
server/plugins/types/UserGraph.ts (1)
UserGraph(1-31)server/plugins/types/AuthorCommits.ts (1)
AuthorCommits(1-5)
🪛 Biome (2.1.2)
styles/output.css
[error] 1508-1508: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
min-width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (21)
CLAUDE.md (1)
1-98: LGTM! Comprehensive coding guidelines documentation.This new documentation file provides clear conventions for the project including development commands, project structure, and Tailwind-based styling guidelines. The dark mode patterns, color system documentation, and component guidelines will help maintain consistency across the codebase.
styles/output.css (1)
1506-1509: Auto-generated Tailwind CSS - no action needed.The static analysis hint about duplicate
min-widthproperties is a false positive. The-moz-fit-contentfollowed byfit-contentis standard cross-browser CSS generated by Tailwind for vendor prefix support. This is expected behavior in generated CSS output.server/services/githubService.ts (4)
49-78: Good implementation of startDate validation.The validation logic properly handles malformed
startDateinputs by checking for finite values and valid month ranges (1-12), with a sensible fallback to 12 months ago. This addresses the previous review feedback.
438-461: Good batching strategy with per-item error handling.The batching approach (3 concurrent requests at a time) with individual error handling that falls back to zero values is robust. This prevents a single failed month from breaking the entire request.
547-611: Good cache protection for custom date queries.The conditional gating (
if (!startDate)) prevents custom date range queries from overwriting the cached default data in the database. This maintains cache integrity while still allowing flexible date-based queries.
822-844: Clean public API with proper error handling.The
getMonthlyContributionfunction properly normalizes the login, delegates to the internal implementation, and handles errors gracefully by logging context and returning an empty array.app/[lang]/leaderboards/apiRoutes.ts (1)
1-2: LGTM! Clean API route constants.Centralizing API route constants promotes consistency and makes refactoring easier. These are appropriately consumed by the leaderboard components.
app/[lang]/leaderboards/page.tsx (1)
38-79: Well-structured responsive layout.The updated layout properly integrates the
TopTierUserscomponent with horizontal scrolling and responsive breakpoints. The flex layout withmin-w-0andflex-1ensures proper overflow handling.app/[lang]/leaderboards/TopTierUsers.tsx (3)
23-42: Good data fetching pattern with error handling.The
useEffectproperly handles the fetch lifecycle with loading state, error handling, and cleanup viafinally. The error is logged for debugging while the UI gracefully handles the failure.
44-65: Nice loading skeleton implementation.The skeleton loading state with pulse animation provides good UX feedback while data is being fetched. The structure mirrors the final rendered content, preventing layout shift.
82-131: LGTM! Well-designed user tile cards.The user tiles have appropriate accessibility attributes (
target="_blank",rel="noopener noreferrer"), good hover interactions with scale transitions, and proper truncation for long usernames. The use of Next.jsImagecomponent ensures optimized image loading.app/[lang]/stats/[login]/Scouter/StatsDetails/StatsChart.tsx (3)
1-24: LGTM! Well-structured imports and type definitions.The component is properly marked as client-side, imports are organized, and the
Propstype clearly defines the expected data shape with optional properties.
36-42: Good hydration mismatch prevention.Using
requestAnimationFrameto defersetIsClient(true)is a solid approach to avoid React 19 hydration warnings while ensuring the chart only renders client-side.
130-216: Clean chart implementation with good UX.The loading state visual feedback (opacity + blur), responsive width calculation, and interactive legend behavior are well implemented.
app/[lang]/leaderboards/GithubUserList.tsx (1)
77-125: Good use of useMemo for column definitions.The column definitions are properly memoized with appropriate dependencies, and the table structure is well-organized.
locales/ko.json (2)
96-105: Localization updates look consistent.The Korean translations for the new leaderboards section are well-structured and align with the English counterparts.
120-124: Stats localization additions look good.The new stats-related Korean translations (
selectPeriod,periodDescription,resetToDefault,search) are properly added and provide clear user-facing text.app/[lang]/stats/[login]/Scouter/StatsDetails/SectionDooboo.tsx (2)
138-148: Good integration of MonthPicker and StatsChart.The MonthPicker is positioned between scores and chart with proper props, and StatsChart receives the monthly contributions data with loading state.
199-215: Clean component composition with endDate prop forwarding.The destructuring pattern separating
endDatefrom other props and forwarding it explicitly toSectionHeaderis clear and maintainable.locales/en.json (2)
87-96: Leaderboards localization section looks good.The new keys (
all,topUsers,topRanked) and updated text (noRecentList→ "No users found") are well-structured.
111-115: Stats localization additions are complete.The new period selection and search-related strings are properly added and provide clear guidance for users.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/[lang]/stats/[login]/SearchTextInput.tsx (1)
112-116: aria-label doesn't differentiate loading and idle states.Both branches of the ternary use
t.search, so screen-reader users won't know the button is searching. Update to provide distinct feedback:- aria-label={ - isLoading - ? t.search || 'Search' - : t.search || 'Search' - } + aria-label={ + isLoading + ? 'Searching...' + : t.search || 'Search' + }If
translations.searchingor a similar key exists in the localization files, prefer that over the hardcoded string.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/[lang]/leaderboards/GithubUserList.tsx(1 hunks)app/[lang]/stats/[login]/SearchTextInput.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/[lang]/stats/[login]/SearchTextInput.tsx (1)
src/hooks/useSearchHistory.ts (1)
useSearchHistory(26-65)
🔇 Additional comments (6)
app/[lang]/leaderboards/GithubUserList.tsx (2)
51-75: Past issue resolved: error handling implemented correctly.The response status check and error handling have been properly implemented as suggested in the previous review. The code now verifies
response.okbefore parsing JSON and handles errors gracefully.
130-167: Past issue resolved: error handling and state management improved.The scroll handler now properly wraps the async fetch in try-catch and uses a functional state updater to avoid stale closure issues. The cursor logic is also correctly managed.
app/[lang]/stats/[login]/SearchTextInput.tsx (4)
33-34: Language extraction assumes Next.js route structure.The extraction
pathname?.split('/')[1]correctly handles theapp/[lang]/stats/[login]/structure. The fallback to'en'is appropriate for undefined pathnames. Given Next.js routing guarantees, the[lang]segment should always be present, making this implementation safe.
37-50: LGTM! State synchronization and loading reset are well-implemented.Both effects correctly address the previous review feedback:
- Lines 37-39 keep the input synchronized with route changes
- Lines 42-50 use
requestAnimationFrameto defer the loading state reset, avoiding ESLint violations while ensuring the reset happens only when navigation completesThe ref-based pattern properly tracks pending navigation and prevents race conditions.
52-61: LGTM! Navigation logic is correct and handles edge cases.The guard at lines 54-56 correctly prevents the loading state from getting stuck when searching for the same user. The language-aware path construction and ref-based tracking ensure smooth navigation between stats pages.
125-129: LGTM! Spinner implementation follows project conventions.The spinner correctly uses Tailwind's
animate-spinutility with semantic color classes (border-gray-300,border-t-brand) and proper dark-mode variants. This aligns with the project's styling approach seen elsewhere in the codebase.
|
Add support for numbered rank like ranked nth out of total number of users. Also, the scores for each metric is different when an end date has been selected because it calculates the data from 12 months before the specific end date to the specific end date, not from the last 12 months. These scores should not be different when an end date has been selected. |
2 participants
Summary by CodeRabbit
New Features
UI/UX Updates
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.