Yash200691/issue163 - #174
Conversation
Fix/layout bug
…ut-bug Revert "Fix/layout bug"
❌ Deploy Preview for cv-community-dashboard failed.
|
WalkthroughThis pull request refactors the homepage activity feed by removing the PaginatedActivitySection component and inlining its functionality directly into the main page with enriched activity card UI. The layout is adjusted across multiple components—the leaderboard view filters and entries are reflowed horizontally, the navbar receives minor spacing updates, and a new asynchronous Footer component is introduced with data fetching capabilities. The People page data-fetching logic is simplified to always fetch from a single endpoint and display timestamps unconditionally. All leaderboard JSON data files are updated with refreshed timestamps and reorganized user contribution data. Possibly related PRs
🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (19)
components/Leaderboard/LeaderboardView.tsx (2)
222-230: Fixed search input width may cause mobile layout issues.The search input now has a fixed
w-64(256px) width, replacing what was likely a responsive width (sm:w-fullaccording to the summary). On viewports narrower than ~400px, this fixed width combined with the horizontal filter layout (line 218) could cause:
- Horizontal overflow or cramped spacing
- Insufficient room for filter/clear buttons
- Poor mobile user experience
♻️ Consider a responsive width approach
<Input type="text" placeholder="Search contributors..." value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} className=" - pl-9 h-9 w-64 bg-white dark:bg-[#07170f] border border-[#50B78B]/60 dark:border-[#50B78B]/40 text-foreground dark:text-foreground shadow-sm dark:shadow-none outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#50B78B] focus-visible:ring-offset-0 transition-colors + pl-9 h-9 w-full sm:w-64 bg-white dark:bg-[#07170f] border border-[#50B78B]/60 dark:border-[#50B78B]/40 text-foreground dark:text-foreground shadow-sm dark:shadow-none outline-none focus:outline-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#50B78B] focus-visible:ring-offset-0 transition-colors " />Alternatively, use
max-w-64withw-fullto allow the input to shrink on smaller screens while capping the maximum width on larger screens.
342-435: Restore responsive layout for mobile viewports.The leaderboard entry cards now use a fixed horizontal layout (
flex items-center gap-6), removing the previous responsive vertical stacking for small screens. On mobile devices (≤640px), this horizontal layout becomes problematic:
- Rank icon (48px) + Avatar (56px) + gaps + info + points/chart area exceeds typical mobile widths
- Content cramming, excessive truncation, or horizontal scroll
- The Activity Trend Chart (lines 417-425) and total points area may not render well in the constrained horizontal space
♻️ Restore responsive column layout
<CardContent> - <div className="flex items-center gap-6"> + <div className="flex flex-col sm:flex-row sm:items-center gap-6"> {/* Rank */} <div className="flex items-center justify-center size-12 shrink-0">This allows the entry to stack vertically on mobile (better use of space) while maintaining the horizontal layout on larger screens.
public/leaderboard/month.json (3)
762-823: Critical: Duplicate activity entries inflating contributor points.The activities array for user
Nihal4777contains duplicate entries for the same pull request:
- PR #6490 "PR opened": Listed twice (lines 764-771 and 773-780) with identical content
- PR #6490 "PR merged": Listed twice (lines 794-801 and 803-810) with identical content
This duplication results in:
- Incorrect
total_points: 20 (should be lower without duplicates)- Incorrect
PR openedcount: 2 (should be 1 for this PR)- Incorrect
PR mergedcount: 3 (includes duplicates)Impact Analysis
This data integrity issue:
- Unfairly inflates the contributor's leaderboard ranking
- Distorts leaderboard accuracy and fairness
- Suggests a bug in the data aggregation logic that may affect other contributors
- Undermines trust in the leaderboard system
The same PR should only appear once per activity type. Please audit the data generation process to prevent duplicate entries.
1393-1434: Critical: Duplicate PR entries for Agarwalchetan.The activities array contains duplicate entries:
- PR #6492: Listed at lines 1395-1402 and 1415-1422 (exact duplicates)
- PR #6491: Listed at lines 1405-1412 and 1425-1432 (exact duplicates)
Impact:
total_points: 8 (should be 4)PR opened count: 4 (should be 2)This is the same data duplication pattern identified in Nihal4777's entry, confirming a systematic issue in the data aggregation logic.
1967-1987: Critical: Duplicate PR entry for sai-ganesh-1706 confirmed, with systematic duplicates across 11 contributors.PR #6493 "Prevent redundant event rescheduling in simulator event queue" appears twice in the activities array (lines 1968-1975 and 1978-1985), resulting in:
total_points: 4 (should be 2)PR opened count: 2 (should be 1)Verification reveals this is part of a widespread data generation issue affecting 11 contributors:
- senutpal (3 duplicate groups, 39 points affected)
- naman79820 (2 duplicate groups, 37 points affected)
- Atharva7126 (3 duplicate groups, 30 points affected)
- Nihal4777, ghanshyam2005singh, salmoneatenbybear, akshitvigg, himanshujays29, Agarwalchetan, webbsssss, and sai-ganesh-1706 (1 duplicate group each)
All duplicate entries have identical links, timestamps, and metadata, indicating automated data generation failures. The leaderboard statistics are significantly inflated across the board.
public/leaderboard/2week.json (3)
307-348: Duplicate activity entries for user Nihal4777.The activities array contains duplicate entries:
- Two identical "PR opened" entries for PR #6490 (Lines 308-327)
- Two identical "PR merged" entries for PR #6490 (Lines 328-347)
This results in inflated counts (
count: 2for both PR opened and PR merged) and doubled points. If the same PR was opened and merged once, each should havecount: 1.
666-708: Duplicate activity entries for user Agarwalchetan.The activities array contains duplicate entries for the same PRs:
- PR #6492 "Fixed Search Query Timeout Issue" appears twice
- PR #6491 "Fixed NoMethodError in NotifyUser Service" appears twice
The
activity_breakdownshowscount: 4but there are only 2 unique PRs. This inflates the contributor's points from 4 to 8.
1094-1116: Duplicate activity entry for user sai-ganesh-1706.PR #6493 "Prevent redundant event rescheduling in simulator event queue" appears twice in the activities array, resulting in
count: 2andpoints: 4instead ofcount: 1andpoints: 2.public/leaderboard/3week.json (3)
706-746: Duplicate activity entries for user Nihal4777 (same issue as 2week.json).PR #6490 appears twice for both "PR opened" and "PR merged" types, inflating the counts and points.
1148-1190: Duplicate activity entries for user Agarwalchetan (same issue as 2week.json).PRs #6492 and #6491 each appear twice in the activities array.
1627-1649: Duplicate activity entry for user sai-ganesh-1706 (same issue as 2week.json).PR #6493 appears twice in the activities array.
public/leaderboard/week.json (4)
32-73: Duplicate activity entries for user Nihal4777 (consistent across all leaderboard files).PR #6490 appears twice for both "PR opened" and "PR merged" types. This same issue exists in 2week.json and 3week.json, suggesting the duplicates originate from the data generation process.
159-200: Duplicate activity entries for user Agarwalchetan (consistent across all leaderboard files).PRs #6492 and #6491 each appear twice.
532-553: Duplicate activity entry for user sai-ganesh-1706 (consistent across all leaderboard files).PR #6493 appears twice.
1-1077: Fix duplicate PR entries in generateLeaderboard.ts by excluding merged PRs from the "PR opened" search.The GitHub API search queries overlap: the "PRs opened" search (line 197) uses
is:prwhich matches all PRs including merged ones, while the "PRs merged" search (lines 209-212) usesis:pr+is:merged. This causes every merged PR to be counted twice inraw_activities— once as "PR opened" and once as "PR merged".To fix, exclude merged PRs from line 197's query by changing:
`org:${ORG}+is:pr`to:
`org:${ORG}+is:pr+-is:merged`This ensures only open (not yet merged) PRs are counted in the "PR opened" activity, preventing duplicates in the leaderboard data.
public/leaderboard/2month.json (4)
1088-1137: Duplicate activity entries for user "Nihal4777".The activities array contains duplicate entries:
- Lines 1088-1107: "PR opened" for PR #6490 appears twice
- Lines 1118-1137: "PR merged" for PR #6490 appears twice
This could inflate the contributor's point total. The
activity_breakdownshows 2 PRs opened (4 points) and 3 PRs merged (15 points), but the unique activities don't match these counts.
1814-1863: Duplicate activity entries for user "anushkaa-dubey".The activities array contains duplicate entries:
- "PR opened" for mobile-app PR #444 appears twice (lines 1814-1843)
- "Issue opened" for mobile-app issue #443 appears twice (lines 1844-1863)
This affects the accuracy of the leaderboard data.
1886-1925: Duplicate activity entries for user "Agarwalchetan".The activities array lists each PR twice:
- "Fixed Search Query Timeout Issue" (PR #6492) appears twice
- "Fixed NoMethodError in NotifyUser Service" (PR #6491) appears twice
The
activity_breakdownshows 4 PRs opened (8 points), but there are only 2 unique PRs.
2820-2839: Duplicate activity entry for user "sai-ganesh-1706".The same PR #6493 ("Prevent redundant event rescheduling in simulator event queue") is listed twice in the activities array, resulting in 4 points instead of 2.
🤖 Fix all issues with AI agents
In @app/people/page.tsx:
- Around line 6-11: The fetchPeople function lacks try/catch around the fetch
call and can throw on network/DNS errors; wrap the fetch and subsequent res.ok
check in a try/catch inside fetchPeople (or catch the fetch promise) so any
thrown exception returns the same graceful fallback ({ updatedAt: Date.now(),
people: [] }) and optionally logs the error; keep the existing res.ok handling
for non-2xx responses and ensure the function still returns Promise<{ updatedAt:
number; people: any[] }> on errors.
- Line 9: The fetch failure branch currently returns { updatedAt: Date.now(),
people: [] } which misleads the UI and swallows errors; update the failure
handling in the fetch logic that checks res.ok to (1) log the error (use
console.error or the existing logger) including status and response
text/throwable, and (2) return a clear sentinel (e.g., updatedAt: null or
updatedAt: 0) or include an error flag (e.g., error: true) instead of Date.now()
so the UI can omit or show an explicit error message when rendering
people/page.tsx.
In @public/leaderboard/month.json:
- Line 3: The updatedAt field in public/leaderboard/month.json has regressed
(matches recent-activities.json), indicating the generation pipeline is writing
older timestamps; locate every leaderboard JSON write that sets the "updatedAt"
field (e.g., month.json and recent-activities.json) and change the generator so
it derives updatedAt from a single authoritative source (current system time or
the latest event timestamp) and/or uses max(previousUpdatedAt, newTimestamp) to
prevent backward moves; run the generator for all leaderboard files and add a
sanity check that updatedAt is non-decreasing across generated files.
In @README.md:
- Around line 169-170: Remove the extra spaces after the heading hash in the
"Demo Site" heading (the line starting with "## Demo Site") to satisfy MD019
and replace the bare URL on the following line
(https://circuitverse-leaderboard.vercel.app/) with proper markdown link syntax
(e.g., [Demo Site](https://circuitverse-leaderboard.vercel.app/)) so it complies
with MD034.
- Line 167: In the contributing section sentence "For questions, Contant on
slack.", correct the typos by replacing "Contant" with "Contact" and
capitalizing "slack" to "Slack" so the sentence reads "For questions, Contact on
Slack." Update that line in README.md accordingly.
🧹 Nitpick comments (10)
app/page.tsx (2)
133-139: External link button with#fallback may confuse users.When
activity.linkis null/undefined, clicking the external link icon navigates to#(page top) rather than doing nothing. Consider hiding the link button entirely when there's no valid URL:🔧 Suggested fix
-<Link - href={activity.link ?? "#"} - target="_blank" - className="opacity-0 group-hover:opacity-100 transition-opacity text-zinc-400 hover:text-[#50B78B] p-1" -> - <ArrowUpRight className="h-4 w-4" /> -</Link> +{activity.link && ( + <Link + href={activity.link} + target="_blank" + className="opacity-0 group-hover:opacity-100 transition-opacity text-zinc-400 hover:text-[#50B78B] p-1" + > + <ArrowUpRight className="h-4 w-4" /> + </Link> +)}
119-124: Add guard for empty contributor name in avatar fallback.If
contributor_nameandcontributorare both empty strings,.slice(0, 2).toUpperCase()returns an empty string, resulting in a blank avatar fallback.🔧 Suggested fix
<AvatarFallback> - {(activity.contributor_name ?? - activity.contributor) - .slice(0, 2) - .toUpperCase()} + {(activity.contributor_name ?? activity.contributor || "??") + .slice(0, 2) + .toUpperCase()} </AvatarFallback>components/footer.tsx (4)
271-276: External links missingtarget="_blank"andrelattributes.The Privacy Policy and Terms of Service links navigate to external
circuitverse.orgURLs but don't open in new tabs, unlike other external links in this component. This inconsistency may cause users to unexpectedly leave the dashboard.🔧 Suggested fix
-<Link href="https://circuitverse.org/privacy"> +<Link href="https://circuitverse.org/privacy" target="_blank" rel="noopener noreferrer"> Privacy Policy </Link> -<Link href="https://circuitverse.org/tos"> +<Link href="https://circuitverse.org/tos" target="_blank" rel="noopener noreferrer"> Terms of Service </Link>
243-248: Add fallback for missingupdatedAtvalue.If
getUpdatedTime()returns null/undefined, the text renders as "Data last updated " with nothing after it. Consider a fallback:🔧 Suggested fix
<p className="text-xs text-zinc-400 dark:text-zinc-500"> Data last updated{" "} <span className="text-zinc-600 dark:text-zinc-300 font-medium"> - {updatedAt && formatTimeAgo(updatedAt)} + {updatedAt ? formatTimeAgo(updatedAt) : "unknown"} </span> </p>
60-60: InconsistentHintcomponent props.The Facebook
Hintspecifiesside="bottom"(line 60), while otherHintcomponents (YouTube, Twitter, etc.) omit thesideprop. Consider standardizing for consistent tooltip positioning.Also applies to: 77-77
207-218: Consider a different icon for "Releases".The Releases link uses the
Trophyicon, which is already used for the Leaderboard. A more semantically appropriate icon likeTag,Rocket, orPackagefrom lucide-react would better distinguish it.app/people/page.tsx (1)
9-10: Replaceany[]with a typed interface for better type safety.Using
any[]for the people array loses type safety and IDE support. Define a proper interface for the person data structure to enable autocomplete and catch type errors at compile time.🔧 Proposed refactor to add type definitions
Add a Person interface at the top of the file:
import Image from "next/image"; import Link from "next/link"; import { getConfig } from "@/lib/config"; import type { Metadata } from "next"; + +interface Person { + username: string; + name?: string; + avatar_url: string; +}Then update the return types:
async function fetchPeople() { const base = process.env.NEXT_PUBLIC_BASE_URL ?? ""; const res = await fetch(`${base}/api/people`, { cache: "no-store" }); - if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] }; - return res.json() as Promise<{ updatedAt: number; people: any[] }>; + if (!res.ok) return { updatedAt: Date.now(), people: [] as Person[] }; + return res.json() as Promise<{ updatedAt: number; people: Person[] }>; }public/leaderboard/year.json (3)
7877-7888: Keepraw_activities[].pointsconsistently populated for scoring correctness.This block adds
points: 5onPR mergeditems. If consumers compute totals fromraw_activities, ensure all “PR merged” records across the file(s) include points (or the consumer has a safe default), otherwise per-user totals can silently drift.
12198-12264:Teesta-Mukherjeeentry looks internally consistent; keep ordering expectations explicit.Totals and per-day points appear coherent for this entry. If the UI assumes
entriesis sorted (e.g., descendingtotal_points), it’d be good to enforce that in the generator/CI rather than relying on manual ordering.
17912-17940:Me-Priyankentry: ensure schema compatibility (occured_atkey + optional fields).This entry follows the existing
"occured_at"spelling. If any newer code expects"occurred_at", it’ll break only for newly-added data too—consider either normalizing the key in the generator or supporting both in the parser.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.mdapp/layout.tsxapp/page.tsxapp/people/page.tsxcomponents/Leaderboard/LeaderboardView.tsxcomponents/PaginatedActivitySection.tsxcomponents/footer.tsxcomponents/navbar.tsxpublic/leaderboard/2month.jsonpublic/leaderboard/2week.jsonpublic/leaderboard/3week.jsonpublic/leaderboard/month.jsonpublic/leaderboard/recent-activities.jsonpublic/leaderboard/week.jsonpublic/leaderboard/year.json
💤 Files with no reviewable changes (1)
- components/PaginatedActivitySection.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/footer.tsx (3)
types/config.ts (1)
Config(53-58)postcss.config.mjs (1)
config(1-5)lib/utils.ts (1)
formatTimeAgo(41-43)
🪛 LanguageTool
README.md
[grammar] ~167-~167: Ensure spelling is correct
Context: ...ntext in PR description. For questions, Contant on slack. ## Demo Site https://circui...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~167-~167: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ... description. For questions, Contant on slack. ## Demo Site https://circuitverse-le...
(ON_SKYPE)
🪛 markdownlint-cli2 (0.18.1)
README.md
169-169: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
170-170: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (16)
components/navbar.tsx (1)
50-50: LGTM! Minor spacing and sizing refinements.The padding adjustments and icon size changes are consistent across desktop and mobile navigation. These appear to be part of the broader UI refresh.
Also applies to: 79-79, 98-98
app/layout.tsx (1)
63-71: Potential duplicate footer: NewFootercomponent exists but isn't used here.The PR introduces a new async
Footercomponent incomponents/footer.tsxwith richer content (social links, system status, etc.), but this layout still uses an inline footer. If the new component is intended to replace this one, consider importing and using it:+import { Footer } from "@/components/footer";<main className="flex-1">{children}</main> -<footer className="border-t py-6 mt-12"> - <div className="container mx-auto px-4 text-center text-sm text-muted-foreground"> - <p> - © {new Date().getFullYear()}{" "} - {config.org.name}. All rights reserved. - </p> - </div> -</footer> +<Footer config={config} />Note: Since
Footeris async,RootLayoutwould need to remain a Server Component (which it currently is).app/page.tsx (1)
93-174: Activity stream UI implementation looks good.The inlined activity rendering with avatars, contributor info, timestamps, and points badges provides a richer UI compared to the previous
PaginatedActivitySection. The structure is clean with proper accessibility through semantic markup.A couple of minor notes:
- The
select-noneclass prevents text selection which may be intentional for the UI feel- Limiting to 10 activities per group (line 108) is a reasonable default
components/footer.tsx (1)
23-57: Footer component structure looks good.The async server component pattern is appropriate for fetching
updatedTime. The organization branding section with logo, description, and social links is well-structured with proper accessibility attributes (aria-labelon links).components/Leaderboard/LeaderboardView.tsx (3)
218-218: Verify the horizontal filter layout on mobile devices.The filters row now uses a fixed horizontal layout without responsive breakpoints. On smaller screens, the search input (w-64), filter button, and clear button may feel cramped or cause horizontal overflow.
Consider testing on mobile viewports (320px-640px width) to ensure the layout remains usable. If issues arise, you may need to restore responsive stacking (e.g.,
flex-col sm:flex-row) or adjust the search input width dynamically.
417-425: Verify Activity Trend Chart readability on mobile devices.The Activity Trend Chart is now rendered unconditionally within the horizontal entry layout (without the previous small-screen hiding). On mobile viewports, the chart may:
- Be too small to interpret meaningfully
- Contribute to the horizontal cramping issue flagged in line 342-435
- Lack sufficient rendering space for readable sparkline visualization
Test the chart rendering on mobile devices. If legibility is poor, consider either:
- Restoring responsive display logic (e.g.,
hidden sm:blockon the chart container), or- Fixing the parent layout to use vertical stacking on mobile (as suggested in the previous comment), which would give the chart more horizontal space to render properly.
514-514: LGTM: Clean component closure.The end-of-file formatting is clean and consistent.
public/leaderboard/recent-activities.json (1)
2-2: The timestamp is not a code issue;updatedAtcorrectly reflects execution time.The
updatedAtfield usesDate.now()(line 383 ofgenerateLeaderboard.ts), which correctly captures the current time when the script runs. A backward timestamp indicates the script was executed at an earlier time than the previous run—this is expected behavior if the workflow was manually triggered, re-run, or if the scheduled execution was delayed. This is not a caching or script bug; it accurately reflects when the leaderboard data was last generated.public/leaderboard/month.json (1)
1-3376: No issues found. The PR successfully updates the footer component and layout as described. The commit includescomponents/footer.tsx(282+ lines) andapp/layout.tsx(77 lines) alongside leaderboard data files. The JSON files are legitimately tracked in this repository's source control, not auto-generated artifacts.Likely an incorrect or invalid review comment.
public/leaderboard/year.json (2)
7891-8084: Data integrity check passed: no duplicate usernames or total_points mismatches detected.Verification confirms the
senutpalentry (and all other entries) maintain correct invariants—unique usernames across the leaderboard and total_points correctly equal the sums of activity_breakdown, daily_activity, and raw_activities points.
3-3:updatedAttimestamp is correctly formatted and monotonic across period files.The epoch millisecond value 1766643841892 correctly converts to 2025-12-25T06:24:01.892Z. Verification across all leaderboard period files confirms proper monotonicity: year.json has the earliest timestamp (1766643841892), with timestamps increasing through week, 2week, 3week, month, 2month, and finally recent-activities.json (1766643841915)—an expected ordering for broader time periods generated before narrower ones.
public/leaderboard/2week.json (1)
1-1895: Data refresh looks consistent overall.The timestamp update, new contributor blocks (Teesta-Mukherjee, ayuxsh009, rehanshuraj, Me-Priyank), and point reallocations appear structurally valid. The primary concerns are the duplicate activity entries flagged above for Nihal4777, Agarwalchetan, and sai-ganesh-1706.
public/leaderboard/3week.json (1)
1-2775: Data structure and new contributor blocks look good.The new contributor blocks for Teesta-Mukherjee, akritah, and Me-Priyank are structurally valid. The duplicate activity entries noted above should be addressed to ensure accurate leaderboard metrics.
public/leaderboard/2month.json (3)
1703-1784: New contributor entry for "Teesta-Mukherjee" looks correct.The new entry has consistent data:
total_points: 10matchesactivity_breakdown(5 PRs opened × 2 points = 10)daily_activitysum (6 + 2 + 2 = 10) matches total- All activities are unique
4502-4533: New contributor entry for "Me-Priyank" looks correct.The new entry has consistent data:
total_points: 1matchesactivity_breakdown(1 Issue opened × 1 point = 1)- Single activity entry is properly structured
3-3: Timestamp update noted.The
updatedAttimestamp has been refreshed to reflect the latest data update.
| async function fetchPeople() { | ||
| const base = process.env.NEXT_PUBLIC_BASE_URL ?? ""; | ||
| const apiUrl = base ? `${base}/api/people` : "/api/people"; | ||
|
|
||
| try { | ||
| const res = await fetch(apiUrl, { cache: "no-store" }); | ||
| if (!res.ok) { | ||
| console.warn(`Failed to fetch people: ${res.status} ${res.statusText}`); | ||
| return { updatedAt: 0, people: [] as any[] }; | ||
| } | ||
| return res.json() as Promise<{ updatedAt: number; people: any[] }>; | ||
| } catch (error) { | ||
| console.error("Error fetching people:", error); | ||
| return { updatedAt: 0, people: [] as any[] }; | ||
| } | ||
| const res = await fetch(`${base}/api/people`, { cache: "no-store" }); | ||
| if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] }; | ||
| return res.json() as Promise<{ updatedAt: number; people: any[] }>; | ||
| } |
There was a problem hiding this comment.
Add error handling for network failures.
The fetch call on line 8 is not wrapped in a try/catch block. If the fetch throws due to network errors, DNS failures, or other exceptions, the entire page render will fail. Since you already have graceful error handling on line 9 for non-ok responses, extend it to catch fetch exceptions as well.
🛡️ Proposed fix to handle fetch exceptions
async function fetchPeople() {
const base = process.env.NEXT_PUBLIC_BASE_URL ?? "";
+ try {
const res = await fetch(`${base}/api/people`, { cache: "no-store" });
if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] };
return res.json() as Promise<{ updatedAt: number; people: any[] }>;
+ } catch (error) {
+ console.error("Failed to fetch people:", error);
+ return { updatedAt: Date.now(), people: [] as any[] };
+ }
}📝 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.
| async function fetchPeople() { | |
| const base = process.env.NEXT_PUBLIC_BASE_URL ?? ""; | |
| const apiUrl = base ? `${base}/api/people` : "/api/people"; | |
| try { | |
| const res = await fetch(apiUrl, { cache: "no-store" }); | |
| if (!res.ok) { | |
| console.warn(`Failed to fetch people: ${res.status} ${res.statusText}`); | |
| return { updatedAt: 0, people: [] as any[] }; | |
| } | |
| return res.json() as Promise<{ updatedAt: number; people: any[] }>; | |
| } catch (error) { | |
| console.error("Error fetching people:", error); | |
| return { updatedAt: 0, people: [] as any[] }; | |
| } | |
| const res = await fetch(`${base}/api/people`, { cache: "no-store" }); | |
| if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] }; | |
| return res.json() as Promise<{ updatedAt: number; people: any[] }>; | |
| } | |
| async function fetchPeople() { | |
| const base = process.env.NEXT_PUBLIC_BASE_URL ?? ""; | |
| try { | |
| const res = await fetch(`${base}/api/people`, { cache: "no-store" }); | |
| if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] }; | |
| return res.json() as Promise<{ updatedAt: number; people: any[] }>; | |
| } catch (error) { | |
| console.error("Failed to fetch people:", error); | |
| return { updatedAt: Date.now(), people: [] as any[] }; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @app/people/page.tsx around lines 6 - 11, The fetchPeople function lacks
try/catch around the fetch call and can throw on network/DNS errors; wrap the
fetch and subsequent res.ok check in a try/catch inside fetchPeople (or catch
the fetch promise) so any thrown exception returns the same graceful fallback ({
updatedAt: Date.now(), people: [] }) and optionally logs the error; keep the
existing res.ok handling for non-2xx responses and ensure the function still
returns Promise<{ updatedAt: number; people: any[] }> on errors.
| return { updatedAt: 0, people: [] as any[] }; | ||
| } | ||
| const res = await fetch(`${base}/api/people`, { cache: "no-store" }); | ||
| if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] }; |
There was a problem hiding this comment.
Log errors and avoid misleading timestamps on failure.
When the fetch fails (line 9), returning Date.now() for updatedAt is misleading because the UI will display "Updated: [current time]" even though no data was successfully fetched. Additionally, errors are silently swallowed with no logging, making production issues difficult to diagnose.
📊 Proposed fix to add logging and handle timestamp properly
Option 1: Log error and return a sentinel value for updatedAt
async function fetchPeople() {
const base = process.env.NEXT_PUBLIC_BASE_URL ?? "";
const res = await fetch(`${base}/api/people`, { cache: "no-store" });
- if (!res.ok) return { updatedAt: Date.now(), people: [] as any[] };
+ if (!res.ok) {
+ console.error(`Failed to fetch people: ${res.status} ${res.statusText}`);
+ return { updatedAt: 0, people: [] as any[] };
+ }
return res.json() as Promise<{ updatedAt: number; people: any[] }>;
}Then conditionally render the timestamp on line 41:
- <p className="text-xs text-muted-foreground mt-2">
- Updated: {new Date(updatedAt).toLocaleString()}
- </p>
+ {updatedAt > 0 && (
+ <p className="text-xs text-muted-foreground mt-2">
+ Updated: {new Date(updatedAt).toLocaleString()}
+ </p>
+ )}Option 2: Show an explicit error message
Alternatively, return an error flag and display a user-friendly message when data fetch fails.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @app/people/page.tsx at line 9, The fetch failure branch currently returns {
updatedAt: Date.now(), people: [] } which misleads the UI and swallows errors;
update the failure handling in the fetch logic that checks res.ok to (1) log the
error (use console.error or the existing logger) including status and response
text/throwable, and (2) return a clear sentinel (e.g., updatedAt: null or
updatedAt: 0) or include an error flag (e.g., error: true) instead of Date.now()
so the UI can omit or show an explicit error message when rendering
people/page.tsx.
| { | ||
| "period": "month", | ||
| "updatedAt": 1766665720557, | ||
| "updatedAt": 1766643841903, |
There was a problem hiding this comment.
Timestamp regression affects multiple leaderboard files.
The updatedAt timestamp in this file also moved backward to 1766643841915 (Dec 24, 2025 21:10 UTC), matching the regression in recent-activities.json. This consistency suggests a systematic issue in the data generation process.
Please verify that the data generation pipeline correctly handles timestamp continuity across all leaderboard JSON files.
🤖 Prompt for AI Agents
In @public/leaderboard/month.json at line 3, The updatedAt field in
public/leaderboard/month.json has regressed (matches recent-activities.json),
indicating the generation pipeline is writing older timestamps; locate every
leaderboard JSON write that sets the "updatedAt" field (e.g., month.json and
recent-activities.json) and change the generator so it derives updatedAt from a
single authoritative source (current system time or the latest event timestamp)
and/or uses max(previousUpdatedAt, newTimestamp) to prevent backward moves; run
the generator for all leaderboard files and add a sanity check that updatedAt is
non-decreasing across generated files.
| 3. Commit with conventional messages | ||
| 4. Open a PR | ||
| Please keep changes small and add context in PR description. For questions, Contact on slack. | ||
| Please keep changes small and add context in PR description. For questions, Contant on slack. |
There was a problem hiding this comment.
Fix typo and capitalization in contributing section.
Line 167 contains two text quality issues:
- "Contant" should be "Contact"
- "slack" should be "Slack" (proper noun)
✏️ Proposed fix
-Please keep changes small and add context in PR description. For questions, Contant on slack.
+Please keep changes small and add context in PR description. For questions, Contact us on Slack.📝 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.
| Please keep changes small and add context in PR description. For questions, Contant on slack. | |
| Please keep changes small and add context in PR description. For questions, Contact us on Slack. |
🧰 Tools
🪛 LanguageTool
[grammar] ~167-~167: Ensure spelling is correct
Context: ...ntext in PR description. For questions, Contant on slack. ## Demo Site https://circui...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~167-~167: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ... description. For questions, Contant on slack. ## Demo Site https://circuitverse-le...
(ON_SKYPE)
🤖 Prompt for AI Agents
In @README.md at line 167, In the contributing section sentence "For questions,
Contant on slack.", correct the typos by replacing "Contant" with "Contact" and
capitalizing "slack" to "Slack" so the sentence reads "For questions, Contact on
Slack." Update that line in README.md accordingly.
| ## Demo Site | ||
| [https://cv-community-dashboard.netlify.app/](https://cv-community-dashboard.netlify.app/) | ||
| https://circuitverse-leaderboard.vercel.app/ |
There was a problem hiding this comment.
Fix markdown formatting violations.
Line 169 has multiple spaces after the heading hash (MD019), and line 170 has a bare URL without markdown link syntax (MD034). These should follow standard markdown formatting:
✏️ Proposed fix
-## Demo Site
-https://circuitverse-leaderboard.vercel.app/
+## Demo Site
+
+[https://circuitverse-leaderboard.vercel.app/](https://circuitverse-leaderboard.vercel.app/)📝 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.
| ## Demo Site | |
| [https://cv-community-dashboard.netlify.app/](https://cv-community-dashboard.netlify.app/) | |
| https://circuitverse-leaderboard.vercel.app/ | |
| ## Demo Site | |
| [https://circuitverse-leaderboard.vercel.app/](https://circuitverse-leaderboard.vercel.app/) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
169-169: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
170-170: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In @README.md around lines 169 - 170, Remove the extra spaces after the heading
hash in the "Demo Site" heading (the line starting with "## Demo Site") to
satisfy MD019 and replace the bare URL on the following line
(https://circuitverse-leaderboard.vercel.app/) with proper markdown link syntax
(e.g., [Demo Site](https://circuitverse-leaderboard.vercel.app/)) so it complies
with MD034.
|
Hey @Yash200691 no title , no description , extra files added. closing pr for now :)) |
2 participants
Description
Briefly describe what this PR does.
Related Issue
Fixes # (issue number)
Type of change
Checklist
Screenshots (if applicable)
Summary by CodeRabbit
Release Notes
New Features
Documentation
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.