Feat: implement multi-view home dashboard with repository-wise metrics - #189
Feat: implement multi-view home dashboard with repository-wise metrics#189Atharva7126 wants to merge 12 commits into
Conversation
✅ Deploy Preview for cv-community-dashboard ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughServer-side app/page.tsx now gathers data (totalMonth, week, month, previousMonthCount, bucketData, config and reposOverview via lib/db.getReposOverview) into an overviewData object and returns the client component HomeDashboard with that payload. A new client component components/home-dashboard.tsx renders Overview and Repositories tabs (summary cards, repo cards, metrics). lib/db.getReposOverview reads public/leaderboard/overview.json and returns RepoStats[], and scripts/generateLeaderboard.ts adds the RepoStats type and a generateRepoOverview flow that writes public/leaderboard/overview.json. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @components/home-dashboard.tsx:
- Around line 173-179: The "Total Contributions" SummaryCard is showing only
activeRepo.totalContributions (misleading); either rename the label to reflect
"Most Active Repo Contributions" or compute the real aggregate and use it:
create a totalContributions const by summing repos via repos.reduce((sum, r) =>
sum + r.current.currentTotalContribution, 0) and pass value={totalContributions}
to the SummaryCard (target the SummaryCard usage and the
activeRepo.totalContributions reference to update).
In @scripts/generateLeaderboard.ts:
- Around line 564-582: In fetchPRsMerge, the bot-filter condition is inverted:
currently it continues when a PR author is NOT a bot, which excludes human PRs;
change the check so the loop skips when isBotUser(pr.user) is true (i.e.,
continue for bots) so only human PRs are counted, updating the condition at the
line containing if (!isBotUser(pr.user)) continue; accordingly.
🧹 Nitpick comments (6)
lib/db.ts (1)
227-243: Add error handling for JSON parse to prevent server crashes.
JSON.parsecan throw on malformed JSON. Other functions in this file (e.g.,getUpdatedTimeat lines 163-172) wrap JSON parsing in try-catch. Apply the same pattern here for consistency and robustness.♻️ Suggested fix
export async function getReposOverview(): Promise<RepoStats[]> { const filePath = path.join( process.cwd(), "public", "leaderboard", `overview.json` ); if (!fs.existsSync(filePath)) return []; - const file = fs.readFileSync(filePath, "utf-8"); - const data = JSON.parse(file); - - if (!data?.repos?.length) return []; - - return data.repos; + try { + const file = fs.readFileSync(filePath, "utf-8"); + const data = JSON.parse(file); + + if (!data?.repos?.length) return []; + + return data.repos; + } catch { + return []; + } }components/home-dashboard.tsx (3)
26-30: Consider adding proper TypeScript types instead ofany.The
overviewDataprop and several component props useanytype, which bypasses TypeScript's type checking. SinceRepoStatsis already exported from@/scripts/generateLeaderboard, you could define a proper interface:♻️ Suggested type definition
import { RepoStats } from "@/scripts/generateLeaderboard"; import { ActivityGroup, MonthBuckets } from "@/lib/db"; type OverviewData = { totalMonth: number; week: ActivityGroup[]; month: ActivityGroup[]; previousMonthCount: number; bucketData: MonthBuckets; config: { org: { name: string; description: string } }; reposData: { reposOverview: RepoStats[]; }; }; export default function HomeDashboard({ overviewData, }: { overviewData: OverviewData; }) { // ... }
202-204: Use a guaranteed unique key for repository cards.
repo.namecan benullper theRepoStatstype. Consider usingrepo.html_urlwhich is guaranteed to be unique and non-null.-<RepoCard key={repo.name} repo={repo} /> +<RepoCard key={repo.html_url} repo={repo} />
315-319: External links should open in new tab with proper security attributes.The repository link points to an external GitHub URL. For better UX and security, external links should open in a new tab with
rel="noopener noreferrer".♻️ Suggested fix
<div className="text-lg font-bold text-zinc-900 dark:text-zinc-100 hover:text-[#50B78B] dark:hover:text-[#50B78B] transition-colors truncate"> - <Link href={repo.html_url}> + <Link href={repo.html_url} target="_blank" rel="noopener noreferrer"> {repo.name} </Link> </div>scripts/generateLeaderboard.ts (2)
471-491: Type syntax cleanup:number | 0is redundant.The union
number | 0is redundant since0is already anumber. Simplify to justnumber.♻️ Suggested fix
export type RepoStats = { name: string | null, description: string | null, language: string | null, avatar_url: string, html_url: string, - stars: number | 0, - forks: number | 0, + stars: number, + forks: number, current: { pr_opened: number, pr_merged: number, issue_created: number, - currentTotalContribution: number | 0, + currentTotalContribution: number, }, // ... }
513-535: Missing rate limiting infetchAllcould trigger GitHub API limits.Other fetch functions use
smartSleepfor rate limiting (e.g.,fetchRepoPRsat line 375), butfetchAlldoesn't. Since this function is used for paginated fetches across multiple repos, it could quickly exhaust the API rate limit.♻️ Add rate limiting
async function fetchAll(url: string) { let page = 1; let results: any[] = []; while (true) { const res = await fetch(`${url}&per_page=100&page=${page}`, { headers: { Authorization: `Bearer ${TOKEN}`, Accept: "application/vnd.github+json", }, }); if (!res.ok) break; + await smartSleep(res, 500); const data = await res.json(); results.push(...data); if (data.length < 100) break; page++; } return results; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/page.tsxcomponents/home-dashboard.tsxlib/db.tspublic/leaderboard/overview.jsonscripts/generateLeaderboard.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
📚 Learning: 2026-01-07T10:51:39.554Z
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
Applied to files:
app/page.tsx
🧬 Code graph analysis (2)
lib/db.ts (1)
scripts/generateLeaderboard.ts (1)
RepoStats(471-491)
app/page.tsx (2)
lib/db.ts (5)
ActivityGroup(21-26)getRecentActivitiesGroupedByType(89-148)getPreviousMonthActivityCount(209-225)getMonthlyActivityBuckets(177-207)getReposOverview(227-243)components/home-dashboard.tsx (1)
HomeDashboard(26-213)
🔇 Additional comments (8)
public/leaderboard/overview.json (1)
1-426: LGTM!This is a generated static data file that matches the
RepoStatstype schema defined inscripts/generateLeaderboard.ts. The data structure is consistent across all repository entries.lib/db.ts (1)
3-3: LGTM!Import correctly updated to include
RepoStatstype which is used as the return type for the newgetReposOverviewfunction.components/home-dashboard.tsx (3)
69-213: LGTM!The dashboard structure, tab switching logic, and layout are well-implemented. The responsive grid layouts and conditional rendering based on
activeTabstate work correctly.
217-298: LGTM!
SummaryCardis a well-structured presentational component with appropriate glassmorphism styling and decoration variants.
300-416: LGTM!
RepoCardandMetricFlowItemcomponents are well-implemented with proper variant-based styling for the metrics display.scripts/generateLeaderboard.ts (2)
596-646: LGTM!The
generateRepoOverviewfunction correctly aggregates repository metrics and handles missing metadata gracefully by skipping repos where meta fetch fails.
878-878: LGTM!Integration point correctly calls
generateRepoOverview()at the end of the year generation flow.app/page.tsx (1)
1-41: LGTM!Clean separation of server-side data fetching and client-side rendering. The
overviewDatabundle consolidates all necessary data for the dashboard in a well-organized structure. This follows proper Next.js App Router patterns for server/client component composition.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @components/home-dashboard.tsx:
- Line 25: Move the RepoStats type out of the build script into a shared types
module and update imports: create a new types file (e.g., types/repos.ts)
exporting the RepoStats type, change this component to import RepoStats from
that shared module instead of scripts/generateLeaderboard, and update
scripts/generateLeaderboard to import RepoStats from the new shared module as
well so both runtime components and build scripts reference the same centralized
type.
In @scripts/generateLeaderboard.ts:
- Around line 596-646: generateRepoOverview currently lets any error during repo
processing bubble up and crash the whole run; wrap the per-repository work
inside the for loop in a try-catch so failures in fetchRepoMeta,
fetchIssuesCreated, fetchPRsOpened, fetchPRsMerge, etc. do not abort processing
(log the repo name and error and continue), and also add a top-level try-catch
around the function body to catch unexpected errors so writeRepoOverview(res)
still runs with whatever partial results were collected (or ensure
writeRepoOverview is called from a finally block), including clear error logging
that includes the repo identifier and error details.
- Around line 493-496: The module-level date constants NOW, CURRENT_START,
PREVIOUS_START and PREVIOUS_END are computed at import time and can become
stale; move their computation into generateRepoOverview() so they are evaluated
when the function runs (or replace them with functions that compute values on
demand), and update any helpers that currently read those globals (e.g., daysAgo
usage, functions referenced inside generateRepoOverview) to accept the computed
dates as parameters so all date-dependent logic uses the fresh per-call values.
- Line 878: generateRepoOverview() is an async function but is invoked without
awaiting its result, so the script can exit before overview.json is written;
update the call site to await generateRepoOverview() and wrap the call in a
try-catch so any errors are logged/handled (e.g., using console.error or the
existing logger) to prevent the whole leaderboard generation from crashing;
locate the invocation of generateRepoOverview and replace it with an awaited,
error-handled call.
🧹 Nitpick comments (5)
components/home-dashboard.tsx (3)
80-108: Eliminate duplicate sorting logic.The same sorting operation is performed twice: once inside
getActiveRepoStats(lines 89-93) and again forsortedRepos(lines 104-108). This is inefficient and violates the DRY principle.♻️ Proposed refactor
Compute
sortedReposonce and reuse it:- function getActiveRepoStats() { - if (!repos || repos.length === 0) { - return { - name: "N/A", - growth: 0, - totalContributions: 0, - }; - } - - const topRepo = [...repos].sort( - (a, b) => - b.current.currentTotalContribution - - a.current.currentTotalContribution - )[0]; - - return { - name: topRepo?.name, - growth: topRepo?.growth?.pr_merged ?? 0, - totalContributions: - topRepo?.current.currentTotalContribution, - }; - } - const activeRepo = getActiveRepoStats(); - const sortedRepos = [...repos].sort( (a, b) => b.current.currentTotalContribution - a.current.currentTotalContribution ); + + function getActiveRepoStats() { + if (sortedRepos.length === 0) { + return { + name: "N/A", + growth: 0, + totalContributions: 0, + }; + } + + const topRepo = sortedRepos[0]; + return { + name: topRepo.name, + growth: topRepo.growth?.pr_merged ?? 0, + totalContributions: topRepo.current.currentTotalContribution, + }; + } + const activeRepo = getActiveRepoStats();
349-355: Consider a more descriptive alt text fallback.The alt text fallback
"Image"is not very descriptive. Consider using a more informative fallback that helps with accessibility.♻️ Suggested improvement
<Image src={repo.avatar_url} - alt={repo.name ?? "Image"} + alt={repo.name ?? "Repository avatar"} width={40} height={40} />
404-429: LGTM with optional type safety improvement.The component logic is sound. The type assertion on line 428 is safe given the
MetricVarianttype constraint, but you could eliminate it with const assertion on thevariantStylesobject.♻️ Optional: Remove type assertion
const variantStyles = { neutral: { icon: "text-zinc-400", count: "text-zinc-700 dark:text-zinc-300", bg: "hover:bg-zinc-100 dark:hover:bg-zinc-700/50", }, active: { icon: "text-blue-500/70 dark:text-blue-400/70", count: "text-zinc-800 dark:text-zinc-200", bg: "hover:bg-blue-50 dark:hover:bg-blue-900/20", }, success: { icon: "text-[#50B78B]", count: "text-[#50B78B] font-bold", bg: "hover:bg-[#50B78B]/10", }, - }; - const styles = - variantStyles[variant as keyof typeof variantStyles]; + } as const satisfies Record<MetricVariant, { icon: string; count: string; bg: string }>; + const styles = variantStyles[variant];scripts/generateLeaderboard.ts (2)
500-511: Add error logging in fetchRepoMeta.The function returns
nullon error without logging the error details, making debugging difficult when repository metadata fetching fails.🔧 Suggested improvement
async function fetchRepoMeta(repo:string) { const res = await fetch(`${GITHUB_API}/repos/${ORG}/${repo}`, { headers: { Authorization: `Bearer ${TOKEN}`, Accept: "application/vnd.github+json", } }); - if (!res.ok) return null; + if (!res.ok) { + console.error(` ⚠️ Failed to fetch metadata for ${repo}: ${res.status}`); + return null; + } await smartSleep(res, 300) return res.json() }
513-535: Replaceany[]with a more specific type.Using
any[]reduces type safety and makes the code harder to maintain. Consider using a more specific type or at leastunknown[].♻️ Suggested improvement
-async function fetchAll(url: string) { +async function fetchAll<T = unknown>(url: string): Promise<T[]> { let page = 1; - let results: any[] = []; + let results: T[] = []; while (true) { const res = await fetch(`${url}&per_page=100&page=${page}`, {This preserves flexibility while providing better type safety at call sites.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/home-dashboard.tsxscripts/generateLeaderboard.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
🧬 Code graph analysis (1)
components/home-dashboard.tsx (7)
lib/db.ts (2)
ActivityGroup(21-26)MonthBuckets(34-39)types/config.ts (1)
Config(53-58)scripts/generateLeaderboard.ts (1)
RepoStats(471-491)lib/utils.ts (1)
cn(5-7)components/Leaderboard/stats-card/activity-line-card.tsx (1)
ActivityLineCard(139-314)components/Leaderboard/stats-card/activity-types.tsx (1)
ActivityTypes(44-148)components/PaginatedActivitySection.tsx (1)
PaginatedActivitySection(32-286)
🔇 Additional comments (2)
scripts/generateLeaderboard.ts (2)
539-582: LGTM! Bot filtering is correctly implemented.The metric fetcher functions correctly filter out bot users using
isBotUser()and properly handle date range filtering for current and previous periods. This ensures contribution stats reflect only human activity as intended.
584-594: LGTM!The function correctly writes the repository overview data with appropriate metadata (timestamp and period).
| import { PaginatedActivitySection } from "@/components/PaginatedActivitySection"; | ||
| import { ActivityGroup, MonthBuckets } from "@/lib/db"; | ||
| import { Config } from "@/types/config"; | ||
| import { RepoStats } from "@/scripts/generateLeaderboard"; |
There was a problem hiding this comment.
Move RepoStats type to a shared types directory.
Importing types directly from a build script (scripts/generateLeaderboard.ts) into a client component creates tight coupling between build-time and runtime code. This violates separation of concerns and makes the codebase harder to maintain.
♻️ Recommended refactor
Create a new shared types file (e.g., types/repos.ts or add to an existing types file) and move the RepoStats type definition there. Then update both this file and scripts/generateLeaderboard.ts to import from the shared location:
-import { RepoStats } from "@/scripts/generateLeaderboard";
+import { RepoStats } from "@/types/repos";Update scripts/generateLeaderboard.ts:
+import { RepoStats } from "@/types/repos";
+
-export type RepoStats = {
- // ... type definition
-}🤖 Prompt for AI Agents
In @components/home-dashboard.tsx at line 25, Move the RepoStats type out of the
build script into a shared types module and update imports: create a new types
file (e.g., types/repos.ts) exporting the RepoStats type, change this component
to import RepoStats from that shared module instead of
scripts/generateLeaderboard, and update scripts/generateLeaderboard to import
RepoStats from the new shared module as well so both runtime components and
build scripts reference the same centralized type.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @scripts/generateLeaderboard.ts:
- Around line 880-881: The call to generateRepoOverview() can throw and
currently bubbles up to fail generateYear() and exit the whole leaderboard run;
wrap the generateRepoOverview() invocation (the call site inside
generateYear()/the end of scripts/generateLeaderboard.ts where you call await
generateRepoOverview()) in a try/catch, log the error (including error details)
via the existing logger, and allow execution to continue (do not rethrow or call
process.exit) so year/week/month artifacts still get produced; if desired, set a
boolean/flag or return value to indicate overview failed for later reporting.
- Around line 471-497: The current window boundaries allow a merge at exactly
CURRENT_START to be counted in both windows and the RepoStats type uses
redundant unions; set PREVIOUS_END = CURRENT_START and change the
previous-window upper-bound check to be exclusive (< PREVIOUS_END instead of <=
PREVIOUS_END) wherever merges are filtered (so events at CURRENT_START are only
in current), and simplify RepoStats by removing pointless unions like number | 0
(use number) and change name: string | null to name: string to match usage.
🧹 Nitpick comments (1)
scripts/generateLeaderboard.ts (1)
584-648: Make overview generation more robust (directory, ordering, typing).
writeRepoOverview()assumespublic/leaderboardexists (currently true viagenerateYear(), but easy to break if reused).metaisany; a missingowner/field will crash the whole repo’s processing.- Consider sorting
res(e.g., bycurrent.currentTotalContribution) to keep output stable for the UI.Proposed diff
function writeRepoOverview(repo:RepoStats[]) { + const outDir = path.join(process.cwd(), "public", "leaderboard"); + fs.mkdirSync(outDir, { recursive: true }); fs.writeFileSync( - path.join(process.cwd(), "public", "leaderboard", "overview.json"), + path.join(outDir, "overview.json"), JSON.stringify({ updatedAt: Date.now(), period: "Last_30days", - repos: repo + repos: repo }, null, 2) ); console.log(`✅ Generated overview.json (${repo.length} repos)`); }- writeRepoOverview(res); + res.sort((a, b) => b.current.currentTotalContribution - a.current.currentTotalContribution); + writeRepoOverview(res);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generateLeaderboard.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
| async function fetchIssuesCreated(repo: string) { | ||
| console.log(" 🔎 Fetching current issues..."); | ||
| const issues = await fetchAll( | ||
| `${GITHUB_API}/repos/${ORG}/${repo}/issues?state=all&since=${iso(CURRENT_START)}` | ||
| ); | ||
|
|
||
| return issues.filter( | ||
| i => | ||
| !i.pull_request && | ||
| new Date(i.created_at) >= CURRENT_START && | ||
| !isBotUser(i.user) | ||
| ).length; | ||
| } | ||
|
|
||
| async function fetchPRsOpened(repo: string) { | ||
| console.log(" 🔎 Fetching current PRs opened..."); | ||
| const prs = await fetchAll( | ||
| `${GITHUB_API}/repos/${ORG}/${repo}/pulls?state=all&sort=created&direction=desc` | ||
| ); | ||
|
|
||
| return prs.filter( | ||
| pr => new Date(pr.created_at) >= CURRENT_START && (!isBotUser(pr.user)) | ||
| ).length; | ||
| } | ||
|
|
||
| async function fetchPRsMerge(repo:string) { | ||
| console.log(" 🔎 Fetching PRs merged..."); | ||
| const prs = await fetchAll( | ||
| `${GITHUB_API}/repos/${ORG}/${repo}/pulls?state=closed&sort=updated&direction=desc` | ||
| ); | ||
|
|
||
| let current = 0; | ||
| let previous = 0; | ||
|
|
||
| for (const pr of prs) { | ||
| if (!pr.merged_at) continue; | ||
| if (isBotUser(pr.user)) continue; | ||
| const mergedAt = new Date(pr.merged_at); | ||
| if (mergedAt >= CURRENT_START && mergedAt <= NOW) current++; | ||
| if (mergedAt >= PREVIOUS_START && mergedAt <= PREVIOUS_END) previous++; | ||
| } | ||
|
|
||
| return { current, previous }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the actual code at the specified location
cd scripts && wc -l generateLeaderboard.ts && head -590 generateLeaderboard.ts | tail -55Repository: CircuitVerse/community-dashboard
Length of output: 1677
🌐 Web query:
GitHub Search API count PRs merged issues created date range best practice official documentation
💡 Result:
Short answer — best practices and official guidance:
-
Use the Search API query qualifiers: is:pr is:merged merged:YYYY-MM-DD..YYYY-MM-DD for merged PR counts, and is:issue created:YYYY-MM-DD..YYYY-MM-DD for issues created in a date range. Construct queries the same way as on GitHub.com and pass them to the search endpoint (q=...). [1][2]
-
Expect the Search API to return total_count, incomplete_results, and be subject to the Search-specific rate limits — authenticate your requests to get higher limits and watch X-RateLimit headers. Use per_page (max 100) + pagination when you need items. [2]
-
Important limitation: GitHub’s search result set is effectively capped for practical retrieval (historically you can only page the first ~1,000 results), so total_count from search may be an estimate and you cannot reliably enumerate arbitrarily-large result sets with a single search. To get exact counts beyond that, use repository-scoped REST endpoints (list issues / list pulls) with date filtering + pagination, GraphQL, or time-slicing your queries (split the date range into smaller windows). [3][4]
-
Recommended workflow for accurate counts over ranges:
- Try Search API first for quick estimates (use qualifiers above). [1][2]
- If you need exact counts or >1,000 results, call repository-specific REST endpoints (List issues, List pull requests) or use GraphQL and paginate through results or split the date range into smaller windows (e.g., per-day / per-week) to avoid the search cap. [4][3]
References:
- Search/finding issues & PRs (qualifiers and date filters). [1] https://docs.github.com/en/enterprise-server@latest/issues/tracking-your-work-with-issues/using-issues/filtering-and-searching-issues-and-pull-requests
- Search API usage, parameters and rate-limits (REST / Octokit examples). [2] https://octokit.github.io/rest.js (search.issuesAndPullRequests / REST search docs)
- Practical 1,000-results/search-cap limitation and time-slicing workaround (community/research references). [3] (discussion / MSR papers noting the 1,000-results limit)
- Repository REST endpoints for exact listing/pagination (issues & pulls). [4] https://docs.github.com/en/rest/issues
If you want, I can: (A) give exact example queries and curl/octokit/GraphQL snippets for counts over a specific date range, or (B) help split a date range into safe time slices and provide code to iterate and sum counts. Which do you want?
🌐 Web query:
GitHub Search API total_count accuracy limits caveats official
💡 Result:
Summary — GitHub Search API total_count accuracy, limits, and caveats
Key points
- total_count is returned by the Search API but is not guaranteed exact for large result sets; results can be truncated by timeouts and the response may set incomplete_results: true. Relying on total_count as a precise, stable count is unsafe. [1][4]
- The Search API enforces a practical cap: you cannot page through more than ~1000 results for a single query (many community reports and discussions; the API/UI show different reachable counts). To get beyond that you must narrow the query (date ranges, other filters) and run multiple queries. [2][3][5]
- Search has custom rate limits and time limits. Slow/expensive queries may time out and return partial matches; those cases set incomplete_results = true. Respect X-RateLimit headers (search uses lower per-minute limits). [1]
- API results (indexing) may differ from GitHub web UI search because indexing state, sync timing, and different search backends can produce different counts and items. Expect transient variation. [3][5]
Practical recommendations
- Treat total_count as an estimate, not an authoritative population count. Check incomplete_results; if true, consider splitting the query (by date, repo, language, etc.) and re-running smaller queries to build an accurate aggregate.
- Page only up to the available limit (per_page up to 100) and design for the 1000-results ceiling by slicing the search space.
- Monitor X-RateLimit-* headers and back off on 403/rate-limit responses.
- For stable, repeatable extraction use narrow predicates and deterministic pagination; for very large sets, aggregate with multiple bounded queries.
Sources
- Official GitHub Search API docs (timeouts, incomplete_results, rate limits). [1]
- Community/issue reports on the 1000-results limit and behavior. [2][4][5]
References
[1] GitHub Search API docs — timeouts, incomplete_results, rate limits.
[2] Issue/discussion: “There is a limit of 1000 results per search.” (PyGithub/GitHub threads).
[3] Issue: API vs web search differences — indexing and result-count divergence.
[4] Community discussion reporting total_count variability / incomplete results behavior.
[5] Additional community threads on 1000-result cap and practical workarounds.
Would you like me to fetch the exact doc URLs or show example code to page/slice queries?
🌐 Web query:
GitHub GraphQL search API count issues PRs per repository date range performance
💡 Result:
Short answer and recommendations:
-
Limits to know
- GraphQL enforces a query “points” rate limit (typically 5,000 points/hr for a user) plus secondary limits and a 500k node-per-call validation; include rateLimit in queries to monitor usage. (docs.github.com)
- The GitHub search API (REST or GraphQL search field) returns at most 1,000 result items for any single search query (total_count can be larger, but you cannot page past the first 1,000). (docs.github.com)
-
Counting issues/PRs per repo in a date range — practical approaches
-
Repository-level connections (recommended when you can iterate repos)
- Use repository.issues (IssueConnection) and repository.pullRequests connections. IssueFilters supports a since (DateTime) filter for “updated at or after” and other filters (assignee, createdBy, states). You can request totalCount for a connection and paginate. This is efficient per-repo and avoids the 1,000-search-result limit. (docs.github.com)
- Caveat: issues.since is “updated at or after” (not strictly “created at”), so getting counts for a created-date range may require additional filtering client-side or fetching items and filtering by createdAt field.
-
Search queries (useful when you need cross-repo counts or want to use created: qualifiers)
- Use the search API (GraphQL search(...) or REST /search/issues) with qualifiers like repo:, is:issue/is:pr, created:, updated: to restrict a date range. But each search result set is capped to the first 1,000 returned items — to cover more than 1,000 matches you must split the date range (e.g., per-day or per-hour buckets) and run multiple queries. (docs.github.com)
-
-
Performance and cost trade-offs
- GraphQL query complexity and node count affect “points” cost; deep queries and requesting many fields increase cost and risk hitting node/compute limits — request only totalCount where possible, keep first/last <=100 and paginate. (docs.github.com)
- If you must compute counts across many repositories or long date ranges, prefer:
- Iterating repos and using repository-level connections with since + totalCount (batch and paginate repos to spread cost). (docs.github.com)
- Or run search queries split into narrow date buckets so each search returns <=1000 items; aggregate counts across buckets. (stackoverflow.com)
- Use exponential backoff, limit concurrency (≤100 concurrent requests), and include rateLimit in your queries to avoid secondary limits. (docs.github.com)
-
Quick implementation tips
- For per-repo created-date counts: fetch issues/pullRequests connections and page items, filtering client-side by createdAt if the API lacks a created-date filter on the connection. Use totalCount only when it matches your filter (e.g., states/since); otherwise page and count. (docs.github.com)
- For large cross-repo queries, break the date range into slices (day/hour) so each search query’s result set <1,000, then sum counts. (stackoverflow.com)
If you want, tell me: (a) how many repositories, (b) whether you need created vs updated date counts, and (c) whether you prefer GraphQL or REST — I’ll draft a sample GraphQL query and a date-slicing plan sized to your rate limits.
Citations:
- 1: https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api?utm_source=openai
- 2: https://docs.github.com/enterprise-cloud%40latest/graphql/reference/queries?utm_source=openai
- 3: https://docs.github.com/graphql/reference/input-objects?utm_source=openai
- 4: https://docs.github.com/enterprise-cloud%40latest/graphql/reference/queries?utm_source=openai
- 5: https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api?utm_source=openai
- 6: https://docs.github.com/graphql/reference/input-objects?utm_source=openai
- 7: https://stackoverflow.com/questions/61810553/how-to-get-more-than-1000-search-results-with-api-github?utm_source=openai
- 8: https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api?utm_source=openai
- 9: https://docs.github.com/en/enterprise-server%403.16/graphql/reference/input-objects?utm_source=openai
- 10: https://stackoverflow.com/questions/61810553/how-to-get-more-than-1000-search-results-with-api-github?utm_source=openai
Performance issue confirmed: add early-stop pagination and prefer GraphQL repository connections over Search API.
The current implementation fetches all PRs/issues without server-side time filtering, causing unnecessary API calls and rate-limit exhaustion. This is a real inefficiency.
However, refine the solution approach:
- Search API
total_countis unreliable: GitHub's official documentation statestotal_countis an estimate (not exact),incomplete_resultscan indicate truncation/timeouts, and practical result pagination is capped at ~1,000 items. Relying on it for counts is unsafe. - Recommended approaches per GitHub docs (in priority order):
- GraphQL repository-level connections (best for per-repo counts): Query
repository.issues/repository.pullRequestswith createdAt filters and requesttotalCounton the connection. Avoids the 1,000-result search limit and is accurate. - REST repository endpoints with pagination + date filtering: List issues/pulls per repo, filter by date server-side where supported (
sinceparameter), and paginate through results. Early-stop when crossing your date threshold (as shown in the review's minimal approach). - Time-sliced search queries (if cross-repo search needed): Split the date range into smaller windows (per-day/per-week) and aggregate counts; each slice ≤1,000 results.
- GraphQL repository-level connections (best for per-repo counts): Query
The early-stop pagination approach shown is valid but incomplete—it optimizes one function. Consider migrating to GraphQL repository connections for all three metric fetchers for cleaner, more efficient queries.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/Leaderboard/LeaderboardView.tsx (1)
455-463: Remove commented-out dead code.The
updateRolesParamfunction is no longer used since its logic has been inlined intotoggleRole(lines 364-383). Commented-out code should be removed rather than left in the codebase, as version control preserves the history if needed.🧹 Proposed cleanup
- // const updateRolesParam = (roles: Set<string>) => { - // const params = new URLSearchParams(searchParams.toString()); - // if (roles.size > 0) { - // params.set("roles", Array.from(roles).join(",")); - // } else { - // params.delete("roles"); - // } - // if (typeof window !== 'undefined') window.history.replaceState(null, '', `${pathname}?${params.toString()}`); - // };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/ThemeSelector.tsxcomponents/Leaderboard/LeaderboardCard.tsxcomponents/Leaderboard/LeaderboardView.tsxcomponents/people/ContributorCard.tsxcomponents/people/ContributorDetail.tsxcomponents/people/PeopleStats.tsx
💤 Files with no reviewable changes (2)
- components/people/ContributorDetail.tsx
- components/people/ContributorCard.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
📚 Learning: 2026-01-07T10:51:39.554Z
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
Applied to files:
components/Leaderboard/LeaderboardView.tsx
🔇 Additional comments (6)
app/ThemeSelector.tsx (1)
15-15: Good cleanup of unused variable.Removing the unused
themevariable from the destructuring is a clean refactor that improves code quality without affecting functionality.components/Leaderboard/LeaderboardCard.tsx (1)
226-226: LGTM! Excellent adherence to naming conventions.Prefixing the unused
activityNameparameter with an underscore in the filter callbacks is a standard best practice that improves code readability and satisfies linting rules. The filter predicate only usesdata.count, and the subsequent.map()callbacks correctly define their ownactivityNameparameter in their respective scopes.Also applies to: 314-314, 455-455, 550-550
components/Leaderboard/LeaderboardView.tsx (3)
29-29: LGTM! Clean removal of unused import.The
useRouterimport was correctly removed as the component now consistently useswindow.history.replaceState()for URL parameter updates throughout. This aligns with the retrieved learning that client-side URL management with proper guards provides better UX in this codebase.
636-636: LGTM! Correct use of unused parameter convention.Renaming the parameter from
eto_ecorrectly signals that the event parameter is intentionally unused in the handler. This follows the standard convention for marking unused function parameters.
811-811: LGTM! Correct removal of unused parameter.The
indexparameter was correctly removed from themapcallback since it's not used anywhere in the function body (lines 812-825). The rank is computed from the pre-calculatedentryRanksmap instead, which ensures consistent ranking across pagination.components/people/PeopleStats.tsx (1)
177-181: No changes needed. The code correctly implements Next.js Image with the project's configuration.The
unoptimized: truesetting innext.config.tsdisables optimization requirements, making thewidthandheightprops optional. Additionally, the GitHub avatar domain (avatars.githubusercontent.com) is already configured in theremotePatternsarray, so the Image component will load properly. This implementation is correct and follows the project's Next.js settings.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lib/db.ts:
- Around line 227-243: The getReposOverview function currently reads and
JSON.parse()s overview.json without guarding against malformed JSON; wrap the
file read and JSON.parse call in a try-catch, catch any parsing or read errors,
log the error (or use an existing logger) and return an empty array on failure
to avoid crashing the app; mirror the defensive pattern used in
getRecentActivitiesGroupedByType so the function returns [] when the file is
missing or corrupted and includes a clear error log referencing
getReposOverview.
🧹 Nitpick comments (3)
lib/db.ts (1)
28-32: Consider removing commented-out type if no longer needed.If
RecentActivitiesJSONis permanently deprecated, consider removing the commented block to keep the codebase clean.scripts/generateLeaderboard.ts (2)
499-510: Consider logging errors in fetchRepoMeta for better observability.The function silently returns
nullon API failures. While the caller logs when the fetch fails, adding error logging here would help diagnose API issues during script execution.📊 Suggested enhancement
async function fetchRepoMeta(repo:string) { const res = await fetch(`${GITHUB_API}/repos/${ORG}/${repo}`, { headers: { Authorization: `Bearer ${TOKEN}`, Accept: "application/vnd.github+json", } }); - if (!res.ok) return null; + if (!res.ok) { + console.error(` ⚠️ Failed to fetch meta for ${repo}: ${res.status}`); + return null; + } await smartSleep(res, 300) return res.json() }
586-596: Add error handling for file write operation.While the directory is created earlier in the flow, wrapping the file write in a try-catch would make this function more resilient and provide clearer error messages if the write fails due to permissions or disk space issues.
🛡️ Proposed enhancement
function writeRepoOverview(repo:RepoStats[]) { + try { fs.writeFileSync( path.join(process.cwd(), "public", "leaderboard", "overview.json"), JSON.stringify({ updatedAt: Date.now(), period: "Last_30days", repos: repo }, null, 2) ); console.log(`✅ Generated overview.json (${repo.length} repos)`); + } catch (error) { + console.error(`❌ Failed to write overview.json:`, error); + throw error; + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/db.tsscripts/generateLeaderboard.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
🧬 Code graph analysis (1)
lib/db.ts (1)
scripts/generateLeaderboard.ts (1)
RepoStats(471-491)
🔇 Additional comments (8)
lib/db.ts (1)
3-3: LGTM!The import additions are appropriate for the new
getReposOverviewfunction that returnsRepoStats[].scripts/generateLeaderboard.ts (7)
471-495: LGTM with note on time constants.The
RepoStatstype is well-structured. The module-level time constants (NOW,CURRENT_START,PREVIOUS_START) are appropriate for a script that runs and exits. If this module were ever used in a long-running process, these would need to be computed at invocation time.
512-537: LGTM!The
fetchAllhelper has good error handling, proper pagination logic, and appropriate rate limiting withsmartSleep.
541-553: LGTM!The function correctly filters out bot users and pull requests, counting only genuine issues created by humans within the 30-day window.
555-564: LGTM!Consistent bot filtering and date range validation ensure accurate PR metrics.
566-584: LGTM!The date range logic correctly separates current (last 30 days) and previous (30-60 days ago) merged PRs, with proper bot filtering.
598-650: LGTM with resilient error handling.The per-repo try-catch ensures that failures in individual repositories don't stop the entire overview generation. The sequential processing is appropriate given GitHub API rate limits.
882-882: LGTM!The placement of
generateRepoOverview()at the end of the year generation flow ensures the overview is created alongside other leaderboard data.
naman79820
left a comment
naman79820
left a comment
There was a problem hiding this comment.
@Atharva7126 some conflicts in generateLeaderboard and coderabbit suggestions. Thankss :))
|
@naman79820 done :] |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/db.ts (2)
28-32: Consider removing commented-out code.The commented
RecentActivitiesJSONtype appears to be unused. If it's no longer needed, removing it would improve code maintainability.
227-246: LGTM: Error handling implemented correctly.The function properly handles file reading errors with try-catch and returns a safe empty array fallback. The implementation is consistent with other data-fetching functions in this file.
Optional improvements for production robustness
Consider these enhancements for better runtime safety and observability:
Runtime type validation: Add validation to ensure
data.reposconforms to theRepoStats[]structure before returning, especially if the JSON file could be manually edited or corrupted.Enhanced error logging: Replace
console.errorwith structured logging or an error tracking service for better observability in production environments.These are nice-to-have improvements since the current implementation provides safe fallbacks and the JSON file is generated by your own scripts.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/db.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
🧬 Code graph analysis (1)
lib/db.ts (1)
scripts/generateLeaderboard.ts (1)
RepoStats(471-491)
🔇 Additional comments (1)
lib/db.ts (1)
3-3: LGTM: Import updated correctly.The
RepoStatsimport is properly added to support the newgetReposOverviewfunction's return type.
|
still there is merge conflicts @Atharva7126 |
Removed unused issue triaging activities and related types.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/Leaderboard/LeaderboardCard.tsx (1)
566-642: Critical: Syntax errors and merge conflict artifacts block compilation.The desktop layout section contains unresolved merge conflicts and syntax errors that prevent the code from compiling:
- Line 587: Orphaned
>closing tag with no matching opening tag- Lines 588-600: Duplicate nested JSX fragment inside the IconComponent conditional (lines 584-586 already render IconComponent, then lines 588-600 duplicate it)
- Lines 606-642: Entire "Top Activities" section appears to be a duplicate or misplaced fragment
- Structural issue: Lines 566-604 define an "Activity Tags" section but lines 586-600 contain conflicting markup
This aligns with the PR comment that "merge conflicts still exist." The file requires manual conflict resolution.
🔧 Resolution guidance
The desktop layout section (starting line 509) should likely contain only ONE activity display block. Review the git diff to determine which version is correct:
- Keep lines 566-604 (Activity Tags) and remove lines 606-642 (Top Activities), OR
- Keep lines 606-642 (Top Activities) and remove lines 566-604 (Activity Tags)
Then fix the syntax error by removing the orphaned code at lines 586-600 (the duplicate IconComponent rendering inside the conditional).
Expected structure should be:
{/* Activity Tags or Top Activities - pick one */} <div className="space-y-1 flex-1"> {sortActivitiesByPriority(Object.entries(entry.activity_breakdown)) .filter(([_activityName, data]) => data.count > 0) .map(([activityName, data]) => { const style = getActivityStyle(activityName); const IconComponent = style.icon; return ( <div key={activityName} className={cn(/* ... */)}> <div className="flex items-center gap-1.5"> {IconComponent && ( <IconComponent className={cn("w-3 h-3", style.textColor)} /> )} <span className={cn("font-medium truncate", style.textColor)}> {activityName}: </span> </div> <div className="flex items-center gap-1"> <span className="font-semibold">{data.count}</span> <span className={cn("font-bold", style.textColor)}>+{data.points}</span> </div> </div> ); })} </div>components/Leaderboard/LeaderboardView.tsx (1)
637-730: Critical: Missing closing tag breaks JSX structure.The
<div className="space-y-4">opened at line 640 is never closed before the</PopoverContent>tag at line 730. This syntax error will prevent the component from rendering.🐛 Proposed fix
Add the missing closing
</div>tag after the Role section and before</PopoverContent>:))} </div> </div> + </div> </PopoverContent>The correct structure should be:
<PopoverContent> <div className="space-y-4"> {/* Sort By Section */} <div>...</div> {/* Divider */} <div /> {/* Role Section */} <div>...</div> </div> ← Missing this closing tag </PopoverContent>
🧹 Nitpick comments (1)
components/Leaderboard/LeaderboardView.tsx (1)
477-485: Remove commented-out code.The
updateRolesParamfunction has been refactored and its logic inlined intoggleRoleand other functions. Commented-out code should be removed to maintain codebase cleanliness.♻️ Proposed cleanup
- // const updateRolesParam = (roles: Set<string>) => { - // const params = new URLSearchParams(searchParams.toString()); - // if (roles.size > 0) { - // params.set("roles", Array.from(roles).join(",")); - // } else { - // params.delete("roles"); - // } - // if (typeof window !== 'undefined') window.history.replaceState(null, '', `${pathname}?${params.toString()}`); - // }; -
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/Leaderboard/LeaderboardCard.tsxcomponents/Leaderboard/LeaderboardView.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
📚 Learning: 2026-01-07T10:51:39.554Z
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
Applied to files:
components/Leaderboard/LeaderboardView.tsx
🧬 Code graph analysis (1)
components/Leaderboard/LeaderboardCard.tsx (1)
lib/utils.ts (1)
cn(5-7)
🪛 Biome (2.1.2)
components/Leaderboard/LeaderboardView.tsx
[error] 640-640: Expected corresponding JSX closing tag for 'div'.
Opening tag
closing tag
(parse)
components/Leaderboard/LeaderboardCard.tsx
[error] 642-642: Expected an expression but instead found '<'.
Expected an expression here.
(parse)
🔇 Additional comments (3)
components/Leaderboard/LeaderboardCard.tsx (1)
246-246: LGTM! Good practice to indicate unused variables.Prefixing the unused
activityNameparameter with an underscore correctly signals that it's intentionally unused in the filter predicate.Also applies to: 334-334, 569-569, 611-611
components/Leaderboard/LeaderboardView.tsx (2)
658-658: LGTM: Clear unused parameter naming.Using
_einstead ofeclearly indicates the event parameter is intentionally unused, which is a good practice for code readability.
831-831: LGTM: Simplified map callback.Removing the unused
indexparameter from themapcallback improves code clarity while preserving all functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/Leaderboard/LeaderboardCard.tsx (1)
475-475: Apply consistent parameter naming for uniformity.For consistency with the changes at lines 246, 334, and 569, this filter callback should also use
_activityNamesince the parameter is unused.♻️ Suggested consistency fix
- .filter(([activityName, data]) => data.count > 0) + .filter(([_activityName, data]) => data.count > 0)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/Leaderboard/LeaderboardCard.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akshitvigg
Repo: CircuitVerse/community-dashboard PR: 139
File: app/leaderboard/page.tsx:1-24
Timestamp: 2026-01-07T10:51:39.554Z
Learning: In the CircuitVerse/community-dashboard Next.js app, client-side redirect with skeleton loading is intentionally used in app/leaderboard/page.tsx instead of server-side redirect() because the server-side redirect causes a blank screen during navigation. The client-side approach with router.replace() and LeaderboardSkeleton provides better UX with immediate visual feedback.
🔇 Additional comments (2)
components/Leaderboard/LeaderboardCard.tsx (2)
246-246: LGTM! Good practice for unused parameters.Prefixing the unused
activityNameparameter with an underscore correctly signals that it's intentionally unused in the filter predicates. This follows best practices and prevents linter warnings.Also applies to: 334-334, 569-569
566-598: LGTM! Activity Tags section is well-implemented.The Activity Tags section is correctly structured with:
- Proper parameter naming (
_activityNamein filter,activityNamein map)- Consistent styling with color-coded activity types
- Responsive layout and proper icon integration
- Correct display of counts and points
naman79820
left a comment
naman79820
left a comment
There was a problem hiding this comment.
Hey @Atharva7126 why did you remove unnecessary changes?
|
just accepted the current changes @naman79820 |
|
Yes you need to add every unnecessary changes that you have removed. The one i sent image was being used to fetch data for issue labeled assigned and closed. |
2 participants
Description
This PR transforms the existing Home page into a comprehensive Home Dashboard by introducing a seamless tabbed interface. It adds a dedicated "Repositories" view to provide granular insights into project-specific activities, while preserving the existing organization-wide "Overview" stats.
Key Changes:
HomeDashboardclient component with a smooth "Overview | Repositories" pill toggle.Issues Created→PRs Opened→PRs Merged) with specific color coding (Neutral/Blue/Green).generateRepoOverviewscript (lib/db.tsor equivalent) to explicitly filter out bot users from Issue and PR counts, ensuring contribution statistics reflect only real human activity.lucide-reacticons and a refined dark/light mode compatible theme using the CircuitVerse green accents.Related Issue
Fixes #180
Type of change
Checklist
Screenshots (if applicable)
Leaderboard.-.CircuitVerse.-.Google.Chrome.2026-01-10.12-53-21.mp4
Summary by CodeRabbit
New Features
Refactor
Bug Fixes / UI
✏️ Tip: You can customize this high-level summary in your review settings.