feat: implement server-side contribution streaks and longest streak tracking#55
feat: implement server-side contribution streaks and longest streak tracking#55vivekyadav-3 wants to merge 13 commits into
Conversation
❌ Deploy Preview for cv-community-dashboard failed.
|
✅ Deploy Preview for circuitverse-leaderboard ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request introduces contribution streak tracking and achievement badge systems across the application. It adds streak calculation utilities that determine current and longest consecutive contribution days based on daily activity data. The API layer is updated to compute and include streak metrics in contributor profiles. New UI components display achievements, activity charts, and enhanced contributor details with server-provided streak data. The leaderboard generation pipeline is expanded to track repository-level statistics, aggregates contribution data, and enriches contributor records with active/stale pull request tracking and streak information. Additionally, monthly activity metrics are refactored to use updated data sources, and a CSS animation is added for visual feedback. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR moves contribution streak calculation from client-side to server-side and introduces tracking for both current and longest streaks on contributor profiles.
Key Changes:
- Added server-side
calculateStreakshelper function to compute current and longest contribution streaks - Updated the API to include streak data in contributor responses
- Modified the UI to display both current streak (🔥) and longest streak (🏆) with visual animations
- Removed client-side streak calculation logic
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/db.ts | Added calculateStreaks function and integrated streak calculation into getContributorProfile |
| app/api/people/route.ts | Added duplicate calculateStreaks function and integrated it into the people API route |
| components/people/ContributorDetail.tsx | Removed client-side streak calculation, now uses server-provided streak data |
| SLACK_MESSAGE_UPDATED.md | Documentation file describing the feature implementation approach |
| SLACK_MESSAGE_DRAFT.md | Draft communication for coordinating with maintainers |
| PULL_REQUEST_DRAFT.md | Detailed PR description template |
| ISSUE_STREAKS_ONLY.md | Feature specification and implementation details |
| GSOC_PROPOSAL_TALKING_POINTS.md | GSoC proposal guidance document |
| GSOC_CONTRIBUTIONS.md | Contribution plan documentation |
| GSOC_APPLICATION_SUMMARY.md | Summary of GSoC contributions |
| GITHUB_ISSUE_DRAFT.md | GitHub issue template for the feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function calculateStreaks(dailyActivity: DailyActivity[]) { | ||
| let currentStreak = 0; | ||
| let longestStreak = 0; | ||
| let tempStreak = 0; | ||
|
|
||
| if (!dailyActivity || dailyActivity.length === 0) return { current: 0, longest: 0 }; | ||
|
|
||
| // Sort chrono | ||
| const sortedDays = [...dailyActivity].sort((a, b) => | ||
| new Date(a.date).getTime() - new Date(b.date).getTime() | ||
| ); | ||
|
|
||
| // Longest | ||
| sortedDays.forEach((day) => { | ||
| if (day.count > 0) { | ||
| tempStreak++; | ||
| if (tempStreak > longestStreak) longestStreak = tempStreak; | ||
| } else { | ||
| tempStreak = 0; | ||
| } | ||
| }); | ||
|
|
||
| // Current | ||
| const today = new Date().toISOString().split("T")[0]; | ||
| const yesterday = new Date(Date.now() - 86400000).toISOString().split("T")[0]; | ||
|
|
||
| const reverseDays = [...sortedDays].reverse(); | ||
| let lastActiveIndex = -1; | ||
|
|
||
| for (let i = 0; i < reverseDays.length; i++) { | ||
| const day = reverseDays[i]; | ||
| if (day && day.count > 0 && (day.date === today || day.date === yesterday)) { | ||
| lastActiveIndex = i; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (lastActiveIndex !== -1) { | ||
| currentStreak = 1; | ||
| for (let i = lastActiveIndex; i < reverseDays.length - 1; i++) { | ||
| const currentDay = reverseDays[i]; | ||
| const nextDay = reverseDays[i + 1]; | ||
|
|
||
| if (!currentDay || !nextDay) break; | ||
|
|
||
| const d1 = new Date(currentDay.date); | ||
| const d2 = new Date(nextDay.date); | ||
| const diffDays = Math.round((d1.getTime() - d2.getTime()) / (1000 * 3600 * 24)); | ||
|
|
||
| if (diffDays === 1 && nextDay.count > 0) { | ||
| currentStreak++; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return { current: currentStreak, longest: longestStreak }; | ||
| } |
There was a problem hiding this comment.
There's code duplication between this calculateStreaks function and the one in lib/db.ts. This violates the DRY (Don't Repeat Yourself) principle and creates a maintenance burden. Consider extracting this into a shared utility module that both files can import.
| period: string; | ||
| updatedAt: number; | ||
| entries: ContributorEntry[]; | ||
| entries: any[]; |
There was a problem hiding this comment.
The type has been changed from the more specific 'ContributorEntry[]' to the less specific 'any[]', reducing type safety. This makes it harder to catch type-related errors at compile time and reduces IDE autocomplete support.
| entries: any[]; | |
| entries: ContributorEntry[]; |
| /** | ||
| * Calculates current and longest contribution streaks | ||
| */ | ||
| function calculateStreaks(dailyActivity: any[]) { |
There was a problem hiding this comment.
Missing edge case validation: The function doesn't check if dailyActivity is empty or null before processing. While the route.ts version includes this check on line 32, this version lacks it, which could lead to errors if called with invalid input.
| function calculateStreaks(dailyActivity: any[]) { | |
| function calculateStreaks(dailyActivity: any[]) { | |
| // Handle missing or empty activity gracefully | |
| if (!Array.isArray(dailyActivity) || dailyActivity.length === 0) { | |
| return { current: 0, longest: 0 }; | |
| } |
| // Calculate Longest Streak | ||
| sortedDays.forEach((day) => { | ||
| if (day.count > 0) { | ||
| tempStreak++; | ||
| if (tempStreak > longestStreak) longestStreak = tempStreak; | ||
| } else { | ||
| tempStreak = 0; |
There was a problem hiding this comment.
The longest streak calculation has a critical bug. It counts all days with activity greater than 0, but doesn't verify they are consecutive. This means if a contributor has activities on days 1, 5, 10, and 15, it will count them as a 4-day streak when they should be treated as separate single-day streaks. The calculation should check if consecutive days in the sorted array are exactly 1 day apart before incrementing the streak.
| // Calculate Longest Streak | |
| sortedDays.forEach((day) => { | |
| if (day.count > 0) { | |
| tempStreak++; | |
| if (tempStreak > longestStreak) longestStreak = tempStreak; | |
| } else { | |
| tempStreak = 0; | |
| // Calculate Longest Streak (requires consecutive calendar days) | |
| let prevActiveDate: Date | null = null; | |
| sortedDays.forEach((day) => { | |
| if (day.count > 0) { | |
| const currentDate = new Date(day.date); | |
| if (prevActiveDate) { | |
| const diffDays = Math.round( | |
| (currentDate.getTime() - prevActiveDate.getTime()) / (1000 * 3600 * 24) | |
| ); | |
| if (diffDays === 1) { | |
| // Consecutive active day, extend the streak | |
| tempStreak++; | |
| } else { | |
| // Non-consecutive active day, start a new streak from this day | |
| tempStreak = 1; | |
| } | |
| } else { | |
| // First active day encountered | |
| tempStreak = 1; | |
| } | |
| if (tempStreak > longestStreak) { | |
| longestStreak = tempStreak; | |
| } | |
| prevActiveDate = currentDate; | |
| } else { | |
| // Inactive day breaks the streak | |
| tempStreak = 0; | |
| prevActiveDate = null; |
| sortedDays.forEach((day) => { | ||
| if (day.count > 0) { | ||
| tempStreak++; | ||
| if (tempStreak > longestStreak) longestStreak = tempStreak; | ||
| } else { | ||
| tempStreak = 0; |
There was a problem hiding this comment.
The longest streak calculation has a critical bug. It counts all days with activity greater than 0, but doesn't verify they are consecutive. This means if a contributor has activities on days 1, 5, 10, and 15, it will count them as a 4-day streak when they should be treated as separate single-day streaks. The calculation should check if consecutive days in the sorted array are exactly 1 day apart before incrementing the streak.
| sortedDays.forEach((day) => { | |
| if (day.count > 0) { | |
| tempStreak++; | |
| if (tempStreak > longestStreak) longestStreak = tempStreak; | |
| } else { | |
| tempStreak = 0; | |
| let lastActiveDate: Date | null = null; | |
| sortedDays.forEach((day) => { | |
| if (day.count > 0) { | |
| const currentDate = new Date(day.date); | |
| if (lastActiveDate) { | |
| const diffDays = Math.round( | |
| (currentDate.getTime() - lastActiveDate.getTime()) / (1000 * 3600 * 24) | |
| ); | |
| if (diffDays === 1) { | |
| tempStreak++; | |
| } else { | |
| tempStreak = 1; | |
| } | |
| } else { | |
| tempStreak = 1; | |
| } | |
| if (tempStreak > longestStreak) { | |
| longestStreak = tempStreak; | |
| } | |
| lastActiveDate = currentDate; | |
| } else { | |
| tempStreak = 0; | |
| lastActiveDate = null; |
| /** | ||
| * Calculates current and longest contribution streaks | ||
| */ | ||
| function calculateStreaks(dailyActivity: any[]) { |
There was a problem hiding this comment.
The type annotation 'any[]' for the dailyActivity parameter reduces type safety. Consider using a proper interface like 'DailyActivity[]' to ensure type safety and better code documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
app/api/people/route.ts (1)
49-51: Potential timezone/DST edge case in date calculations.Using hardcoded
86400000milliseconds assumes all days are exactly 24 hours. During DST transitions, days can be 23 or 25 hours, which could affect streak calculations for users in affected timezones.🔎 Safer date arithmetic using date-fns or native date manipulation
// Current - const today = new Date().toISOString().split("T")[0]; - const yesterday = new Date(Date.now() - 86400000).toISOString().split("T")[0]; + const now = new Date(); + const today = now.toISOString().split("T")[0]; + const yesterdayDate = new Date(now); + yesterdayDate.setDate(yesterdayDate.getDate() - 1); + const yesterday = yesterdayDate.toISOString().split("T")[0];
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
GITHUB_ISSUE_DRAFT.mdGSOC_APPLICATION_SUMMARY.mdGSOC_CONTRIBUTIONS.mdGSOC_PROPOSAL_TALKING_POINTS.mdISSUE_STREAKS_ONLY.mdPULL_REQUEST_DRAFT.mdSLACK_MESSAGE_DRAFT.mdSLACK_MESSAGE_UPDATED.mdapp/api/people/route.tscomponents/people/ContributorDetail.tsxlib/db.ts
🧰 Additional context used
🪛 LanguageTool
PULL_REQUEST_DRAFT.md
[grammar] ~64-~64: Use a hyphen to join words.
Context: ...dern grid layout: Before: - Single column layout - Basic stats display - Li...
(QB_NEW_EN_HYPHEN)
GSOC_CONTRIBUTIONS.md
[style] ~5-~5: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...se Leaderboard Date: December 29, 2025 Demo: https://circuitverse-leader...
(MISSING_COMMA_AFTER_YEAR)
GSOC_APPLICATION_SUMMARY.md
[uncategorized] ~167-~167: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...oft Skills:** - Async communication in open source - Handling overlapping work gracefully ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~172-~172: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...es - Writing clear documentation ### Open Source Process: - Importance of small, focu...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~273-~273: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...** 🚀 --- Last Updated: December 30, 2025 Contact: [Your Email/GitHub]
(MISSING_COMMA_AFTER_YEAR)
SLACK_MESSAGE_UPDATED.md
[style] ~35-~35: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 968 characters long)
Context: ...he Contribution Streaks issue right away! 🚀 Branch: https://github.com/vive...
(EN_EXCESSIVE_EXCLAMATION)
GITHUB_ISSUE_DRAFT.md
[style] ~12-~12: Consider a different adjective to strengthen your wording.
Context: ...racking (similar to GitHub) - Provide deeper insights into contribution patterns -...
(DEEP_PROFOUND)
GSOC_PROPOSAL_TALKING_POINTS.md
[grammar] ~52-~52: Use a hyphen to join words.
Context: ...it's the foundation of maintainable open source projects." --- ### **4. User Ex...
(QB_NEW_EN_HYPHEN)
[grammar] ~142-~142: Use a hyphen to join words.
Context: ...:** "I've experienced firsthand how open source collaboration works—from handling...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
PULL_REQUEST_DRAFT.md
124-124: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
128-128: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
132-132: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
GSOC_CONTRIBUTIONS.md
6-6: Bare URL used
(MD034, no-bare-urls)
326-326: Bare URL used
(MD034, no-bare-urls)
327-327: Bare URL used
(MD034, no-bare-urls)
328-328: Bare URL used
(MD034, no-bare-urls)
329-329: Bare URL used
(MD034, no-bare-urls)
ISSUE_STREAKS_ONLY.md
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
135-135: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
GITHUB_ISSUE_DRAFT.md
92-92: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (9)
lib/db.ts (2)
206-234: Current streak logic looks correct.The calculation properly:
- Finds the most recent activity (today or yesterday) as the starting point
- Walks backward checking consecutive calendar days via
diffDays === 1- Breaks when a gap is detected
This correctly handles activity gaps that aren't represented in the
dailyActivityarray.
239-285: getContributorProfile implementation is well-structured.The function properly:
- Handles missing files and contributors with appropriate defaults
- Transforms raw activities with date parsing
- Aggregates daily activity into a map
- Integrates streak calculation into the returned stats
Once the longest streak bug is fixed, this will provide the correct data shape for downstream consumers.
components/people/ContributorDetail.tsx (2)
34-35: Streak data integration looks good.The optional fields on
ContributorEntryprovide backward compatibility, and the consumption logic on lines 74-76 safely handles undefined values with fallback to 0.Also applies to: 74-76
115-119: Streak UI implementation matches the design.The streak indicators are well-positioned:
- Badge on avatar for active streaks (lines 115-119)
- Stats card showing current/longest with emoji labels (lines 144-147)
The conditional rendering and animation enhance the gamification aspect as intended.
Also applies to: 144-147
GSOC_APPLICATION_SUMMARY.md (1)
1-272: Documentation content looks comprehensive.The GSoC application summary is well-structured and provides good context for the contribution. Minor optional stylistic improvements: consider hyphenating "open-source" when used as a compound adjective (lines 167, 172).
app/api/people/route.ts (2)
6-22: Well-structured type definitions for the new streak feature.The
DailyActivityinterface and extensions toContributorEntryare clean and provide good type safety for the streak data flow.
122-135: Server-side streak calculation integration looks good.The streak calculation is properly integrated into the entry processing loop, and the computed values are correctly spread into the entry object before storage.
PULL_REQUEST_DRAFT.md (2)
95-96: Verify file path reference accuracy.The documentation references
app/[username]/page.tsx, but the streak implementation in this PR appears to be inapp/api/people/route.ts. Ensure the file paths accurately reflect the actual changes.
1-198: Comprehensive PR documentation.The draft provides excellent context with motivation, technical details, testing notes, and collaboration acknowledgments. The structure follows good PR practices.
| _Last Updated: December 30, 2025_ | ||
| _Contact: [Your Email/GitHub]_ |
There was a problem hiding this comment.
Update placeholder contact information before merging.
Line 274 contains placeholder text [Your Email/GitHub] that should be replaced with actual contact information or removed entirely.
🧰 Tools
🪛 LanguageTool
[style] ~273-~273: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...** 🚀 --- Last Updated: December 30, 2025 Contact: [Your Email/GitHub]
(MISSING_COMMA_AFTER_YEAR)
🤖 Prompt for AI Agents
In GSOC_APPLICATION_SUMMARY.md around lines 273 to 274, the contact field
currently contains the placeholder “[Your Email/GitHub]”; replace this
placeholder with real contact information (a valid email address and/or GitHub
handle) or remove the line entirely if no contact should be published, then
ensure formatting matches surrounding italicized metadata and commit the change.
|
|
||
| ## 🔗 Related Issue | ||
|
|
||
| Closes #[ISSUE_NUMBER] _(will update after creating issue)_ |
There was a problem hiding this comment.
Multiple placeholders need completion before PR submission.
This draft contains several placeholders that should be filled in:
- Line 9:
#[ISSUE_NUMBER] - Lines 124, 128, 132: Screenshot placeholders
- Lines 190-192: Link placeholders
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/globals.css (1)
197-204: LGTM! Consider centralizing animations in Tailwind config.The pulse animation implementation is correct and performant. The opacity-based animation with 2s duration provides a subtle, effective visual indicator for active streaks.
Recommended: Centralize custom animations in
tailwind.config.tsfor better maintainability:🔎 Suggested approach to define animations in Tailwind config
In your
tailwind.config.ts:export default { theme: { extend: { keyframes: { 'pulse-slow': { '0%, 100%': { opacity: '1' }, '50%': { opacity: '0.7' } } }, animation: { 'pulse-slow': 'pulse-slow 2s cubic-bezier(0.4, 0, 0.6, 1) infinite' } } } }This allows you to use
animate-pulse-slowdirectly in your JSX without custom CSS, keeping all animation definitions in one place.Optional accessibility enhancement: Consider respecting
prefers-reduced-motion:@media (prefers-reduced-motion: reduce) { .animate-pulse-slow { animation: none; } }This ensures users with motion sensitivity preferences won't see the pulsing effect.
|
Hey @vivekyadav-3 great work, just a few suggestions:
Also I'm only supreeth, tag me as @ShinichiShi 😄 |
|
Hey @ShinichiShi, thanks for the review! I’ve made all the updates you
pointed out:
-
Removed the extra markdown files to keep the codebase tidy.
-
Added clear labels for *Current Streak* and *Longest Streak* so the
icons make more sense.
-
Fixed the CodeRabbit warnings by moving the custom animation into the
Tailwind v4 theme block and adding reduced-motion support for accessibility.
It should be all set for another review!
…On Wed, Dec 31, 2025 at 12:44 PM Supreeth C ***@***.***> wrote:
*ShinichiShi* left a comment (CircuitVerse/community-dashboard#55)
<#55 (comment)>
Hey @vivekyadav-3 <https://github.com/vivekyadav-3> great work, just a
few suggestions:
- remove the markdown files as they are unnecessary
- in the collaborator card I can see 🔥/🏆, i didn't get what 🔥
indicates here ?
- fix the coderabbit mentioned issues
Also I'm only supreeth, tag me as @ShinichiShi
<https://github.com/ShinichiShi> 😄
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BSGQEXGXWNC3WLAPAOVJOA34ENZWZAVCNFSM6AAAAACQL4VAAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBRGYYDKMRYGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
* <http://www.kiit.ac.in>*
*
Go Green: Kindly don't print this unless so
required.*
*
*
*Established U/S 3 of UGC Act and Accredited by NBA of
AICTE and NAAC of UGC*
Visit us @ *http://www.kiit.ac.in
<http://www.kiit.ac.in/>*
Follow us @* **|***Twitter
<https://www.twitter.com/KIITUniversity/>***|***Facebook**
<https://www.facebook.com/KIITUniversity>*|***Instagram
<https://www.instagram.com/KIITUniversity/>***|*
The information
contained in this electronic message and any
attachments to this message
are intended for the exclusive use of the
addressee(s) and may contain
proprietary, confidential or privileged
information. If you are not the
intended recipient, you should not
disseminate, distribute or copy this
e-mail. Please notify the sender
immediately and delete all copies of
this message and any attachments.
VIRUS WARNING: Computer viruses can
be
transmitted via email. The recipient should check this email and any
attachments for the presence of viruses. The institute accepts no liability
for any damage caused by any virus transmitted by this email.
|
|
Hey @vivekyadav-3 can you please resolve coderabbit and copilot suggestions please :)) |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/db.ts (2)
152-156: Consider adding TODO comments or explicit return types for stub functions.These stubs return empty values without indication of their intended future implementation. Adding brief comments or explicit return types would help future maintainers understand the intent.
173-192: Reduceanyusage to improve type safety.Multiple
anytype assertions reduce TypeScript benefits. Consider defining explicit types for the activity data structure.🔎 Proposed fix
+interface RawActivity { + occured_at: string; + points?: number; + [key: string]: unknown; +} + - const activities = (contributor.raw_activities || []).map((a: any) => ({ + const activities = (contributor.raw_activities || []).map((a: RawActivity) => ({ ...a, occured_at: new Date(a.occured_at), })); const dailyActivityMap = new Map<string, { count: number; points: number }>(); - activities.forEach((activity: any) => { + activities.forEach((activity: { occured_at: Date; points?: number }) => { const date = new Date(activity.occured_at).toISOString().split("T")[0]; // ... }); - const dailyActivity: DailyActivity[] = Array.from(dailyActivityMap.entries()).map(([date, stats]: [string, any]) => ({ + const dailyActivity: DailyActivity[] = Array.from(dailyActivityMap.entries()).map(([date, stats]) => ({ date, ...stats }));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/api/people/route.tslib/db.tslib/streak-utils.ts
🧰 Additional context used
🧬 Code graph analysis (2)
app/api/people/route.ts (1)
lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-89)
lib/db.ts (1)
lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - circuitverse-leaderboard
- GitHub Check: Header rules - circuitverse-leaderboard
- GitHub Check: Pages changed - circuitverse-leaderboard
🔇 Additional comments (10)
lib/streak-utils.ts (4)
1-5: LGTM!Clean interface definition that properly types the daily activity data structure.
10-13: LGTM!Good defensive handling of empty or null input arrays.
46-65: LGTM!The current streak logic correctly anchors on today/yesterday and scans backwards for consecutive active days. The early-break optimization avoids unnecessary iteration.
67-86: LGTM!The backward iteration properly checks consecutive days using date arithmetic and breaks on gaps.
app/api/people/route.ts (4)
4-5: LGTM!Good refactoring to use shared streak utilities, addressing the prior DRY concern.
14-16: LGTM!Type safety restored by using
DailyActivity[]and properly typed optional streak fields. This addresses the prior type regression feedback.
19-23: LGTM!
entriesis now properly typed asContributorEntry[], restoring type safety as requested in previous reviews.
54-66: LGTM!Clean integration of server-side streak calculation. The
entryWithStreaksobject correctly enriches the entry with computed streak values before storing in the map.lib/db.ts (2)
145-145: LGTM!Path construction is clean and consistent.
196-206: LGTM!The return object provides a comprehensive profile with properly calculated streaks using the shared utility.
e048119 to
d03a757
Compare
|
Hey @naman79820, thanks for calling those out! I pushed a refactor to cover
everything you mentioned:
-
*Streak logic* is fixed now. It properly checks consecutive days with
actual date math instead of just counting active days.
-
*Code duplication* is gone. I moved the streak calculation into a shared
utility (lib/streak-utils.ts).
-
*Types* are cleaned up and replaced with proper interfaces instead of any
.
-
*History cleanup* is done too. Removed the personal/draft files so the
PR only focuses on the feature.
Should be good for another review
…On Thu, Jan 1, 2026 at 12:31 AM Naman Chhabra ***@***.***> wrote:
*naman79820* left a comment (CircuitVerse/community-dashboard#55)
<#55 (comment)>
Hey @vivekyadav-3 <https://github.com/vivekyadav-3> can you please
resolve coderabbit and copilot suggestions please :))
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BSGQEXGZ72XDZNTT6JOGXIT4EQMQRAVCNFSM6AAAAACQL4VAAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBSG4ZDGNJVGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
* <http://www.kiit.ac.in>*
*
Go Green: Kindly don't print this unless so
required.*
*
*
*Established U/S 3 of UGC Act and Accredited by NBA of
AICTE and NAAC of UGC*
Visit us @ *http://www.kiit.ac.in
<http://www.kiit.ac.in/>*
Follow us @* **|***Twitter
<https://www.twitter.com/KIITUniversity/>***|***Facebook**
<https://www.facebook.com/KIITUniversity>*|***Instagram
<https://www.instagram.com/KIITUniversity/>***|*
The information
contained in this electronic message and any
attachments to this message
are intended for the exclusive use of the
addressee(s) and may contain
proprietary, confidential or privileged
information. If you are not the
intended recipient, you should not
disseminate, distribute or copy this
e-mail. Please notify the sender
immediately and delete all copies of
this message and any attachments.
VIRUS WARNING: Computer viruses can
be
transmitted via email. The recipient should check this email and any
attachments for the presence of viruses. The institute accepts no liability
for any damage caused by any virus transmitted by this email.
|
|
i think 2 of the suggestions done by coderrabit are still pending can you have a look please :)) @vivekyadav-3 |
|
Done! I’ve added the defensive checks and unified the rounding logic as
suggested. Please take another look when you get a chance.
…On Fri, Jan 2, 2026 at 3:04 PM Naman Chhabra ***@***.***> wrote:
*naman79820* left a comment (CircuitVerse/community-dashboard#55)
<#55 (comment)>
i think 2 of the suggestions done by coderrabit are still pending can you
have a look please :)) @vivekyadav-3 <https://github.com/vivekyadav-3>
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BSGQEXCRGR2LDKMOK7KHWQT4EY3RVAVCNFSM6AAAAACQL4VAAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBUHA3DKMZSGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
* <http://www.kiit.ac.in>*
*
Go Green: Kindly don't print this unless so
required.*
*
*
*Established U/S 3 of UGC Act and Accredited by NBA of
AICTE and NAAC of UGC*
Visit us @ *http://www.kiit.ac.in
<http://www.kiit.ac.in/>*
Follow us @* **|***Twitter
<https://www.twitter.com/KIITUniversity/>***|***Facebook**
<https://www.facebook.com/KIITUniversity>*|***Instagram
<https://www.instagram.com/KIITUniversity/>***|*
The information
contained in this electronic message and any
attachments to this message
are intended for the exclusive use of the
addressee(s) and may contain
proprietary, confidential or privileged
information. If you are not the
intended recipient, you should not
disseminate, distribute or copy this
e-mail. Please notify the sender
immediately and delete all copies of
this message and any attachments.
VIRUS WARNING: Computer viruses can
be
transmitted via email. The recipient should check this email and any
attachments for the presence of viruses. The institute accepts no liability
for any damage caused by any virus transmitted by this email.
|
|
i don't think you have pushed it yet @vivekyadav-3 |
|
Apologies for the delay! I've just pushed the fixes for both CodeRabbit suggestions (defensive check in db.ts and unified rounding in streak-utils.ts). Please take another look! |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/db.tslib/streak-utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/streak-utils.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/db.ts (2)
scripts/generateLeaderboard.ts (1)
UserEntry(83-98)lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
🔇 Additional comments (2)
lib/db.ts (2)
183-184: LGTM! Import addresses previous review concerns.Moving
calculateStreaksandDailyActivityto a dedicated utility module improves code organization and resolves the type safety and logic issues flagged in previous reviews.
195-197: LGTM! Defensive check properly added.The validation for
data.entriesbefore calling.find()addresses the previous review feedback and prevents potential runtime errors when entries are undefined or not an array.
|
Hi Naman,
Sorry for the confusion. I had the changes ready but hadn’t pushed the
commit to the remote branch yet.
I’ve now pushed the updates that address both CodeRabbit suggestions:
-
*Defensive check*: Added a guard clause in lib/db.ts to safely handle
data.entries.
-
*Unified rounding*: Switched to Math.round in streak-utils.ts to keep
day-difference calculations consistent.
The PR should be up to date now. Let me know if anything else needs
tweaking.
…On Fri, Jan 2, 2026 at 5:57 PM Naman Chhabra ***@***.***> wrote:
*naman79820* left a comment (CircuitVerse/community-dashboard#55)
<#55 (comment)>
i don't think you have pushed it yet @vivekyadav-3
<https://github.com/vivekyadav-3>
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BSGQEXHLMM74BUFCLS3Z6PT4EZP2ZAVCNFSM6AAAAACQL4VAAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBVGIZDONJYGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
* <http://www.kiit.ac.in>*
*
Go Green: Kindly don't print this unless so
required.*
*
*
*Established U/S 3 of UGC Act and Accredited by NBA of
AICTE and NAAC of UGC*
Visit us @ *http://www.kiit.ac.in
<http://www.kiit.ac.in/>*
Follow us @* **|***Twitter
<https://www.twitter.com/KIITUniversity/>***|***Facebook**
<https://www.facebook.com/KIITUniversity>*|***Instagram
<https://www.instagram.com/KIITUniversity/>***|*
The information
contained in this electronic message and any
attachments to this message
are intended for the exclusive use of the
addressee(s) and may contain
proprietary, confidential or privileged
information. If you are not the
intended recipient, you should not
disseminate, distribute or copy this
e-mail. Please notify the sender
immediately and delete all copies of
this message and any attachments.
VIRUS WARNING: Computer viruses can
be
transmitted via email. The recipient should check this email and any
attachments for the presence of viruses. The institute accepts no liability
for any damage caused by any virus transmitted by this email.
|
|
Two more suggestions have come in from coderabbitai @vivekyadav-3 |
…d property access
|
I've handled the two new suggestions from CodeRabbit! I've unified the return types in getContributorProfile and corrected the username property access in lib/db.ts. I'll monitor for any further suggestions before tagging you or Atharva. Thanks for the heads up! 😊 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/db.ts (2)
152-162: Consider explicit typing in sort callback.The sort callback uses implicit
anytyping. While functionally correct, explicitly typing the parameters asUserEntrywould improve type safety and code clarity.🔎 Proposed refinement
- return data.entries.sort((a: any, b: any) => b.total_points - a.total_points); + return data.entries.sort((a: UserEntry, b: UserEntry) => b.total_points - a.total_points);
208-218: Consider stronger typing for better type safety.Several callbacks use implicit
anytyping. While functionally correct, explicit typing would improve maintainability and catch potential errors at compile time.🔎 Proposed refinement
- const contributor = data.entries.find((entry: any) => entry.username.toLowerCase() === username.toLowerCase()); + const contributor = data.entries.find((entry: UserEntry) => entry.username.toLowerCase() === username.toLowerCase()); - const activities = (contributor.activities || []).map((a: any) => ({ + const activities = (contributor.activities || []).map((a) => ({ ...a, occured_at: new Date(a.occured_at), })); const dailyActivityMap = new Map(); - activities.forEach((activity: any) => { + activities.forEach((activity) => { const date = new Date(activity.occured_at).toISOString().split("T")[0];Note: If you strongly type
contributorasUserEntry, TypeScript will infer types foractivitiesandactivitybased on theUserEntry.activitiesfield definition.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/db.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/db.ts (2)
scripts/generateLeaderboard.ts (1)
UserEntry(83-98)lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
🔇 Additional comments (6)
lib/db.ts (6)
144-150: LGTM!The function correctly reads the update timestamp from week.json with appropriate null handling.
164-171: LGTM!The function correctly transforms leaderboard data to extract contributor avatars and names.
173-176: LGTM!The function correctly extracts usernames from the leaderboard.
178-181: LGTM!The function correctly performs case-insensitive username lookup with appropriate null handling.
183-183: LGTM!Good separation of concerns by moving streak calculation logic to a dedicated utility module.
185-246: Well-structured implementation with proper defensive checks.The function correctly:
- Validates file existence and data structure before processing
- Uses consistent return shape across all exit paths (including
dailyActivityandstats)- Performs case-insensitive username lookup with correct property access
- Aggregates daily activity and delegates streak calculation to the dedicated utility
The previous critical issues flagged in past reviews have been properly addressed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
app/api/people/route.ts (1)
7-18: Type safety gap:activities?: any[]undermines TypeScript guarantees.The PR objectives mention "100% TypeScript with zero
anytypes", butactivitieson Line 17 still usesany[]. Additionally, thisContributorEntryinterface duplicates the one inscripts/generateLeaderboard.ts(UserEntry), which could lead to drift.Consider importing and extending
UserEntryfrom the shared location, or define a properActivitytype:🔎 Proposed fix
+interface Activity { + type: string; + title: string; + occured_at: string; + link: string; + points: number; +} + interface ContributorEntry { username: string; name: string | null; avatar_url: string; role: string; total_points: number; activity_breakdown: Record<string, { count: number; points: number }>; daily_activity: DailyActivity[]; current_streak?: number; longest_streak?: number; - activities?: any[]; + activities?: Activity[]; }components/people/ContributorDetail.tsx (2)
229-246: Extract PR turn-around calculation from JSX render.This inline IIFE performs non-trivial computation (sorting, mapping, date math) on every render. This duplicates logic already present in
lib/db.ts(getContributorProfilecalculatesavgTurnAroundMs).Consider either:
- Expose the pre-computed value from the API and consume it here
- Extract to a
useMemohook to avoid recalculation on each render🔎 Option 2: Extract to useMemo
+ const avgMergeTime = useMemo(() => { + const prOpened = new Map<string, number>(); + const times: number[] = []; + const sorted = [...(contributor.activities || [])].sort( + (a, b) => new Date(a.occured_at).getTime() - new Date(b.occured_at).getTime() + ); + + sorted.forEach(act => { + if (act.type === "PR opened") prOpened.set(act.link, new Date(act.occured_at).getTime()); + else if (act.type === "PR merged" && prOpened.has(act.link)) { + times.push(new Date(act.occured_at).getTime() - prOpened.get(act.link)!); + } + }); + + if (times.length === 0) return "N/A"; + const avgHours = Math.round(times.reduce((a, b) => a + b, 0) / times.length / (1000 * 3600)); + return avgHours > 24 ? `${Math.round(avgHours / 2.4) / 10} days` : `${avgHours}h`; + }, [contributor.activities]); // Then in JSX: - <span className="text-3xl font-bold"> - {(() => { ... })()} - </span> + <span className="text-3xl font-bold">{avgMergeTime}</span>
253-266: Consider: Current streak displayed twice.The current streak value appears both in the stats grid (Line 174) and in the "Activity Impact" card (Line 262). This may be intentional for different visual contexts, but if not, consider consolidating or differentiating the metrics shown.
lib/db.ts (4)
152-162: ReplaceanywithUserEntryin sort callback.The
UserEntrytype is already imported, so the sort comparator can be properly typed:🔎 Proposed fix
- return data.entries.sort((a: any, b: any) => b.total_points - a.total_points); + return data.entries.sort((a: UserEntry, b: UserEntry) => b.total_points - a.total_points);
208-210: UseUserEntrytype instead ofanyin find callback.🔎 Proposed fix
- const contributor = data.entries.find((entry: any) => entry.username.toLowerCase() === username.toLowerCase()); + const contributor = data.entries.find((entry: UserEntry) => entry.username.toLowerCase() === username.toLowerCase());
212-231: Multipleanytypes in activity processing reduce type safety.Consider defining a local type for the normalized activity and using proper typing for the map entries:
🔎 Proposed fix
+type NormalizedActivity = { + type: string; + title?: string | null; + link?: string | null; + points: number; + occured_at: Date; +}; - const activities = (contributor.activities || []).map((a: any) => ({ + const activities: NormalizedActivity[] = (contributor.activities || []).map((a: any) => ({ ...a, occured_at: new Date(a.occured_at), })); - const dailyActivityMap = new Map(); - activities.forEach((activity: any) => { + const dailyActivityMap = new Map<string, { count: number; points: number }>(); + activities.forEach((activity) => { const date = new Date(activity.occured_at).toISOString().split("T")[0]; // ... }); - const dailyActivity: DailyActivity[] = Array.from(dailyActivityMap.entries()).map(([date, stats]: [string, any]) => ({ + const dailyActivity: DailyActivity[] = Array.from(dailyActivityMap.entries()).map(([date, stats]) => ({ date, ...stats }));
183-184: Move import to top of file.The import for
calculateStreaksandDailyActivityis placed mid-file. Standard convention is to group all imports at the top.🔎 Proposed fix
Move this import to the top of the file, after the existing imports:
import { UserEntry } from "@/scripts/generateLeaderboard"; import fs from "fs"; import path from "path"; +import { calculateStreaks, DailyActivity } from "./streak-utils";Then remove lines 183-184.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/api/people/route.tscomponents/people/ContributorDetail.tsxlib/db.ts
🧰 Additional context used
🧬 Code graph analysis (2)
app/api/people/route.ts (1)
lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
lib/db.ts (2)
scripts/generateLeaderboard.ts (1)
UserEntry(83-98)lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
🔇 Additional comments (4)
app/api/people/route.ts (1)
57-68: LGTM on streak calculation integration.The server-side streak calculation is cleanly integrated. The fallback pattern on Line 61 handles both
activitiesand legacyraw_activitiesfields appropriately. The map-based deduplication with higher-points preference is correct.components/people/ContributorDetail.tsx (2)
78-80: LGTM on server-side streak consumption.Clean integration of server-provided streak values with sensible defaults.
171-197: LGTM on streak UI cards.The streak display meets the PR requirements with appropriate icons (🔥 for current, 🏆 for longest), conditional pulse animation for active streaks, and consistent styling.
lib/db.ts (1)
258-269: LGTM on profile return structure.The return object provides a consistent shape with all computed statistics. The streak calculation properly delegates to the shared utility.
Note:
avgTurnAroundMsis computed here but the frontend (ContributorDetail.tsx) recalculates it client-side. Consider exposing this through the API to avoid duplication.
|
All the checks are failing now |
|
Fixed! I've pushed the missing badge system files (AchievementBadges.tsx and badges.ts). The build should pass now. Sorry for the oversight! |
2 similar comments
|
Fixed! I've pushed the missing badge system files (AchievementBadges.tsx and badges.ts). The build should pass now. Sorry for the oversight! |
|
Fixed! I've pushed the missing badge system files (AchievementBadges.tsx and badges.ts). The build should pass now. Sorry for the oversight! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/badges.ts (2)
87-94: Consider documenting the single-badge-per-category behavior.The
else-ifchains ensure only the highest badge in each category (streak, points) is awarded. If this is intentional, consider adding a brief comment to clarify the design choice for future maintainers.
102-104: Replace hardcoded username with a data-driven approach.Hardcoding a specific username for the WoC 5.0 badge reduces maintainability and scalability. Consider moving eligible usernames to a configuration file, database table, or at minimum, a constant array at the top of this file with a comment explaining it's a temporary solution.
🔎 Example refactor
+// TODO: Move to database or config file +const WOC_5_PARTICIPANTS = ["vivekyadav1207vy"]; + export function getContributorBadges(contributor: ContributorForBadges) { // ... rest of function - // Manually add WoC 5.0 for the user (vivekyadav1207vy) - if (contributor.username === "vivekyadav1207vy") { + // Award WoC 5.0 badge to participants + if (contributor.username && WOC_5_PARTICIPANTS.includes(contributor.username)) { addBadge("woc_5"); }components/people/AchievementBadges.tsx (1)
23-23: Consider moving TooltipProvider higher in the component tree.While functional, wrapping
TooltipProvideraround the map creates a new context for each render. For better performance, especially with many badges, consider placing it at the Card or component level.🔎 Example refactor
export function AchievementBadges({ badges }: AchievementBadgesProps) { if (badges.length === 0) return null; return ( + <TooltipProvider> <Card className="..."> {/* ... CardHeader ... */} <CardContent> <div className="flex flex-wrap gap-4"> - <TooltipProvider> {badges.map((badge) => ( <Tooltip key={badge.slug}> {/* ... badge content ... */} </Tooltip> ))} - </TooltipProvider> </div> </CardContent> </Card> + </TooltipProvider> ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/people/AchievementBadges.tsxlib/badges.ts
🧰 Additional context used
🧬 Code graph analysis (1)
components/people/AchievementBadges.tsx (3)
lib/badges.ts (1)
BadgeDefinition(1-8)components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)components/ui/tooltip.tsx (4)
TooltipProvider(61-61)Tooltip(61-61)TooltipTrigger(61-61)TooltipContent(61-61)
🔇 Additional comments (3)
lib/badges.ts (1)
1-72: LGTM! Well-structured badge definitions.The interface and badge definitions are clear, comprehensive, and align well with the PR objectives for streak-based, points-based, and special badges.
components/people/AchievementBadges.tsx (2)
1-11: LGTM! Clean component setup.The imports, interface definition, and early return pattern are well-structured and follow React best practices.
13-56: Well-implemented badge display with good UX.The component provides a polished user experience with:
- Visual hierarchy with Card components
- Interactive tooltips for badge details
- Smooth hover animations
- Good accessibility with
cursor-help- Responsive flexbox layout
| }, | ||
| }; | ||
|
|
||
| export function getContributorBadges(contributor: any) { |
There was a problem hiding this comment.
Replace any with a proper type for type safety.
The contributor parameter is typed as any, which bypasses TypeScript's type checking and could lead to runtime errors. Define or import a proper interface (e.g., ContributorEntry mentioned in the AI summary) that includes the expected properties: current_streak, longest_streak, total_points, role, and username.
🔎 Suggested fix
+interface ContributorForBadges {
+ current_streak?: number;
+ longest_streak?: number;
+ total_points?: number;
+ role?: string;
+ username?: string;
+}
+
-export function getContributorBadges(contributor: any) {
+export function getContributorBadges(contributor: ContributorForBadges) {Alternatively, if a ContributorEntry interface already exists elsewhere in the codebase, import and use that type.
📝 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.
| export function getContributorBadges(contributor: any) { | |
| interface ContributorForBadges { | |
| current_streak?: number; | |
| longest_streak?: number; | |
| total_points?: number; | |
| role?: string; | |
| username?: string; | |
| } | |
| export function getContributorBadges(contributor: ContributorForBadges) { |
🤖 Prompt for AI Agents
In lib/badges.ts around line 74, the function parameter is typed as `any`;
replace it with a concrete type (e.g., `ContributorEntry`) that describes
expected properties to restore type safety. Either import an existing
ContributorEntry interface or declare one locally with the required fields
(current_streak: number, longest_streak: number, total_points: number, role:
string, username: string) and update the function signature to use that type;
make fields optional (?) only if they can legitimately be missing and update
usages accordingly.
|
Fixed! I've pushed the missing badge system files (AchievementBadges.tsx and badges.ts). The build should pass now. Sorry for the oversight! |
1 similar comment
|
Fixed! I've pushed the missing badge system files (AchievementBadges.tsx and badges.ts). The build should pass now. Sorry for the oversight! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/db.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/db.ts (2)
scripts/generateLeaderboard.ts (1)
UserEntry(83-98)lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
🔇 Additional comments (4)
lib/db.ts (4)
158-168: LGTM!The function properly validates
data.entriesbefore accessing it and returns a sorted array. The defensive check on line 165 prevents errors when entries are missing.
170-187: LGTM!Clean helper functions with appropriate use of case-insensitive matching in
getContributor. The delegation togetLeaderboard()ensures consistent data sourcing.
194-239: LGTM!Good use of the
defaultReturnpattern ensuring consistent return shape across all code paths. The defensive checks and proper typing withDailyActivity[]address previous review feedback.
264-275: VerifyactivityByDateMap serialization behavior.Line 268 returns
dailyActivityMapwhich is aMapobject. If this return value is serialized to JSON (e.g., in an API response),Mapobjects become empty objects{}.The
dailyActivityarray on line 269 will serialize correctly, so streak functionality should work. However, if consumers rely onactivityByDate, they may receive an empty object unexpectedly.Consider converting the Map to a plain object if this data needs to be JSON-serialized:
return { contributor, activities, totalPoints: contributor.total_points, - activityByDate: dailyActivityMap, + activityByDate: Object.fromEntries(dailyActivityMap), dailyActivity, stats: { currentStreak: streaks.current, longestStreak: streaks.longest, avgTurnAroundMs } };
|
All conflicts resolved! ✅ The missing badge files have been added and the branch is now cleanly mergeable with main. CI checks are running. Thanks for your patience! |
|
Hi Naman,
Everything’s sorted now:
-
Missing badge files have been added
-
Merge conflicts are resolved
-
CodeRabbit suggestions are implemented
-
The branch merges cleanly with main
The PR is ready for a final review. Sorry again for the earlier confusion
around the missing files. I’ve cleaned up my git workflow to avoid this
going forward.
Thanks for your patience!
…On Fri, Jan 2, 2026 at 7:15 PM Naman Chhabra ***@***.***> wrote:
*naman79820* left a comment (CircuitVerse/community-dashboard#55)
<#55 (comment)>
All the checks are failing now
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BSGQEXFDJBJNEO3Z4HYCQ6T4EZY6TAVCNFSM6AAAAACQL4VAAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBVGM2TGNJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
* <http://www.kiit.ac.in>*
*
Go Green: Kindly don't print this unless so
required.*
*
*
*Established U/S 3 of UGC Act and Accredited by NBA of
AICTE and NAAC of UGC*
Visit us @ *http://www.kiit.ac.in
<http://www.kiit.ac.in/>*
Follow us @* **|***Twitter
<https://www.twitter.com/KIITUniversity/>***|***Facebook**
<https://www.facebook.com/KIITUniversity>*|***Instagram
<https://www.instagram.com/KIITUniversity/>***|*
The information
contained in this electronic message and any
attachments to this message
are intended for the exclusive use of the
addressee(s) and may contain
proprietary, confidential or privileged
information. If you are not the
intended recipient, you should not
disseminate, distribute or copy this
e-mail. Please notify the sender
immediately and delete all copies of
this message and any attachments.
VIRUS WARNING: Computer viruses can
be
transmitted via email. The recipient should check this email and any
attachments for the presence of viruses. The institute accepts no liability
for any damage caused by any virus transmitted by this email.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/people/ContributorDetail.tsx (1)
197-198: Remove unused variables.
displayNameanddisplayUsernameare defined but never used. Lines 233–234 still referencecontributor.name || contributor.usernameand@${contributor.username}directly.🔎 Proposed fix
const earnedBadges = getContributorBadges(contributor); - - const displayName = contributor.name || contributor.username; - const displayUsername = `@${contributor.username}`;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/people/ContributorDetail.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/people/ContributorDetail.tsx (3)
lib/badges.ts (1)
getContributorBadges(74-107)types/db.ts (1)
Activity(21-31)components/people/AchievementBadges.tsx (1)
AchievementBadges(10-57)
🔇 Additional comments (6)
components/people/ContributorDetail.tsx (6)
27-28: LGTM—Badge system imports are correctly integrated.The imports for
AchievementBadgesandgetContributorBadgesalign with the new badge functionality used at lines 195 and 297.
56-57: LGTM—Streak fields properly integrated into the interface.Making
current_streakandlongest_streakoptional ensures backward compatibility while supporting the new server-side streak calculation.
179-181: LGTM—Server-side streak integration is clean and well-documented.The implementation correctly consumes server-provided streak values with appropriate fallbacks, achieving the primary PR objective of moving streak calculation to the backend.
253-267: LGTM—Streak cards are well-designed and accessible.The Current Streak and Longest Streak cards clearly communicate their purpose with distinct icons (🔥 and 🏆) and conditional animation. The pulse effect only activates when there's an active streak, providing good visual feedback.
297-297: LGTM—Badge rendering is cleanly integrated.The
AchievementBadgescomponent is properly positioned and will render the earned badges computed from server-provided streak and points data.
335-348: LGTM—Activity Impact card and Total Actions are correctly implemented.The Activity Impact card cleanly displays the current streak, and line 373's defensive
|| []handling prevents potential runtime errors when activities are undefined.Also applies to: 373-373
| {(() => { | ||
| const prOpened = new Map<string, number>(); | ||
| const times: number[] = []; | ||
| const sorted = [...(contributor.activities || [])].sort( | ||
| (a, b) => new Date(a.occured_at).getTime() - new Date(b.occured_at).getTime() | ||
| ); | ||
|
|
||
| sorted.forEach(act => { | ||
| if (act.type === "PR opened") prOpened.set(act.link, new Date(act.occured_at).getTime()); | ||
| else if (act.type === "PR merged" && prOpened.has(act.link)) { | ||
| times.push(new Date(act.occured_at).getTime() - prOpened.get(act.link)!); | ||
| } | ||
| }); | ||
|
|
||
| if (times.length === 0) return "N/A"; | ||
| const avgHours = Math.round(times.reduce((a, b) => a + b, 0) / times.length / (1000 * 3600)); | ||
| return avgHours > 24 ? `${Math.round(avgHours / 2.4) / 10} days` : `${avgHours}h`; | ||
| })()} |
There was a problem hiding this comment.
Fix the day conversion arithmetic and clarify the logic.
Line 327 contains confusing and potentially incorrect arithmetic: Math.round(avgHours / 2.4) / 10.
- This should simply be
avgHours / 24to convert hours to days. - The current version rounds before the final division, which can introduce inaccuracies. For example, with
avgHours = 50:- Current:
Math.round(50 / 2.4) / 10 = 21 / 10 = 2.1 days - Correct:
50 / 24 = 2.08 days(then format appropriately)
- Current:
Additionally, this complex inline calculation reduces readability and testability.
🔎 Proposed fix—correct arithmetic and extract helper
Option 1: Quick fix (inline)
- return avgHours > 24 ? `${Math.round(avgHours / 2.4) / 10} days` : `${avgHours}h`;
+ return avgHours > 24 ? `${(avgHours / 24).toFixed(1)} days` : `${avgHours}h`;Option 2: Extract to helper function (recommended)
Create a new file lib/pr-metrics.ts:
interface Activity {
type: string;
link: string;
occured_at: string;
}
export function calculateAverageMergeTime(activities: Activity[]): string {
const prOpened = new Map<string, number>();
const times: number[] = [];
const sorted = [...activities].sort(
(a, b) => new Date(a.occured_at).getTime() - new Date(b.occured_at).getTime()
);
sorted.forEach(act => {
if (act.type === "PR opened") {
prOpened.set(act.link, new Date(act.occured_at).getTime());
} else if (act.type === "PR merged" && prOpened.has(act.link)) {
times.push(new Date(act.occured_at).getTime() - prOpened.get(act.link)!);
}
});
if (times.length === 0) return "N/A";
const avgMs = times.reduce((a, b) => a + b, 0) / times.length;
const avgHours = Math.round(avgMs / (1000 * 3600));
if (avgHours > 24) {
const days = (avgHours / 24).toFixed(1);
return `${days} days`;
}
return `${avgHours}h`;
}Then use it in the component:
+import { calculateAverageMergeTime } from "@/lib/pr-metrics";
+
...
- {(() => {
- const prOpened = new Map<string, number>();
- const times: number[] = [];
- const sorted = [...(contributor.activities || [])].sort(
- (a, b) => new Date(a.occured_at).getTime() - new Date(b.occured_at).getTime()
- );
-
- sorted.forEach(act => {
- if (act.type === "PR opened") prOpened.set(act.link, new Date(act.occured_at).getTime());
- else if (act.type === "PR merged" && prOpened.has(act.link)) {
- times.push(new Date(act.occured_at).getTime() - prOpened.get(act.link)!);
- }
- });
-
- if (times.length === 0) return "N/A";
- const avgHours = Math.round(times.reduce((a, b) => a + b, 0) / times.length / (1000 * 3600));
- return avgHours > 24 ? `${Math.round(avgHours / 2.4) / 10} days` : `${avgHours}h`;
- })()}
+ {calculateAverageMergeTime(contributor.activities || [])}🤖 Prompt for AI Agents
In components/people/ContributorDetail.tsx around lines 311 to 328, the day
conversion uses incorrect and confusing arithmetic (Math.round(avgHours / 2.4) /
10) and should be fixed and clarified: compute average in milliseconds first,
derive avgHours = avgMs / (1000*3600) (as a number, not prematurely rounded),
then when avgHours > 24 convert to days with days = (avgHours / 24).toFixed(1)
(or Math.round(days*10)/10) and return `${days} days`; otherwise return
`${Math.round(avgHours)}h`. For clarity and testability, extract this logic into
a helper (e.g., lib/pr-metrics.ts with a calculateAverageMergeTime(activities)
function), use that helper from the component, and ensure activities is
null/undefined guarded before calling.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/db.ts (2)
248-258: Critical: Guard against null/undefined links when matching PRs.This issue was flagged in a previous review but remains unaddressed. If
activity.linkisnullorundefined, multiple different PRs without links will map to the same Map key, causing incorrect turnaround time calculations. Only PRs with valid links should be included in the turnaround calculation.🔎 Proposed fix
sortedActivities.forEach(activity => { - if (activity.type === "PR opened") { + if (activity.type === "PR opened" && activity.link) { prOpenedMap.set(activity.link, activity.occured_at); - } else if (activity.type === "PR merged") { + } else if (activity.type === "PR merged" && activity.link) { const openedAt = prOpenedMap.get(activity.link); if (openedAt) { const diff = activity.occured_at.getTime() - openedAt.getTime(); turnAroundTimes.push(diff); } } });
278-283: Critical: Remove duplicateMonthBucketstype declaration.The
MonthBucketstype is already declared and exported at lines 34-39. This duplicate declaration should be removed to avoid confusion and potential compilation issues.🔎 Proposed fix
-export type MonthBuckets = { - w1: number; - w2: number; - w3: number; - w4: number; -}; -Based on static analysis hints from Biome.
🧹 Nitpick comments (2)
lib/db.ts (2)
158-168: Consider using UserEntry type instead ofanyin sort callback.The sort callback parameters on line 167 use
anytype, which reduces type safety. SinceUserEntryis already imported and available, use it for better type checking and IDE support.🔎 Proposed fix
- return data.entries.sort((a: any, b: any) => b.total_points - a.total_points); + return data.entries.sort((a: UserEntry, b: UserEntry) => b.total_points - a.total_points);
214-214: Consider using specific types instead ofanyfor better type safety.Lines 214, 218, and 224 use
anytype annotations, which reduces type safety. Consider usingUserEntryfor the contributor entry and defining an activity type for better IDE support and type checking.🔎 Proposed fix
- const contributor = data.entries.find((entry: any) => entry.username.toLowerCase() === username.toLowerCase()); + const contributor = data.entries.find((entry: UserEntry) => entry.username.toLowerCase() === username.toLowerCase());For activities, consider defining a type alias if one doesn't already exist, or use the activity type from UserEntry:
- const activities = (contributor.activities || []).map((a: any) => ({ + const activities = (contributor.activities || []).map((a) => ({Also applies to: 218-221
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/db.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/db.ts (2)
scripts/generateLeaderboard.ts (1)
UserEntry(42-50)lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
🪛 Biome (2.1.2)
lib/db.ts
[error] 278-279: Shouldn't redeclare 'MonthBuckets'. Consider to delete it or rename it.
'MonthBuckets' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (5)
lib/db.ts (5)
150-156: LGTM!The
getUpdatedTime()function is well-implemented with proper defensive checks for file existence and conditional Date construction.
170-177: LGTM!Clean implementation that correctly derives a simplified contributor list from the leaderboard.
179-182: LGTM!Straightforward implementation for extracting contributor usernames.
184-187: LGTM!Proper case-insensitive lookup with appropriate null handling.
189-189: LGTM!Import statement correctly brings in the required streak calculation utilities.
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 (1)
components/people/ContributorDetail.tsx (1)
1-19: Critical: File has structural/parse errors preventing compilation.The file is missing essential structure. After "use client"; at line 1, lines 2-19 contain function body code (return statements, if blocks) without any function declaration. Additionally, lines 28 onwards reference undefined variables (
contributor,onBack,getContributorBadges) with no component function wrapper, and there are no imports.The file must include:
- All required imports (React, component libraries, utility functions)
- Function declaration for the getActivityConfig helper (lines 2-19 appear to be its body)
- Component function declaration wrapping the main logic (lines 28-72+), e.g.,
export default function ContributorDetail({ contributor, onBack })
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/people/ContributorDetail.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/people/ContributorDetail.tsx (1)
lib/badges.ts (1)
getContributorBadges(74-107)
🪛 Biome (2.1.2)
components/people/ContributorDetail.tsx
[error] 18-18: Illegal return statement outside of a function
(parse)
[error] 19-19: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🔇 Additional comments (4)
components/people/ContributorDetail.tsx (4)
45-47: LGTM — Server-provided streak consumption is correctly implemented.The shift to server-side streak calculation aligns with the PR objectives. Defensive fallbacks to
0handle missing data gracefully, and the comment clearly documents the data source.
65-65: LGTM — Badge integration is correctly wired.The
getContributorBadgescall properly leverages the server-provided streak data, and the function inlib/badges.tshandles missing fields defensively.
68-69: Display helpers look good.The
displayNamefallback is appropriate. Minor consideration: ifcontributor.usernamecould ever be undefined,displayUsernamewould produce@undefined. If the API guarantees this field, this is fine as-is.
33-36: LGTM — Max points calculation handles empty arrays correctly.The
|| 0fallback properly handles the edge case wheresortedActivitiesis empty (sinceMath.max(...[])returns-Infinity).
fefa893 to
95b15e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/db.ts (1)
248-258: Guard against null/undefined links when matching PRs.If
activity.linkisnullorundefined, multiple PRs without links will map to the same key (undefined), causing incorrect turnaround time calculations. This was flagged in a previous review and remains unaddressed.🔎 Proposed fix
sortedActivities.forEach(activity => { - if (activity.type === "PR opened") { + if (activity.type === "PR opened" && activity.link) { prOpenedMap.set(activity.link, activity.occured_at); - } else if (activity.type === "PR merged") { + } else if (activity.type === "PR merged" && activity.link) { const openedAt = prOpenedMap.get(activity.link); if (openedAt) { const diff = activity.occured_at.getTime() - openedAt.getTime(); turnAroundTimes.push(diff); } } });
🧹 Nitpick comments (4)
tests/lib/streak-utils.test.ts (2)
38-58: Extract duplicatedformatDatehelper to reduce code repetition.The
formatDatefunction is duplicated in three test cases (lines 40-45, 62-67, 86-91). Consider extracting it to the top of the file.🔎 Proposed refactor
import { describe, it, expect } from "vitest"; import { calculateStreaks, DailyActivity } from "../../lib/streak-utils"; +const formatDate = (date: Date) => { + const year = date.getFullYear(); + const month = String(date.getMonth() + 1).padStart(2, '0'); + const day = String(date.getDate()).padStart(2, '0'); + return `${year}-${month}-${day}`; +}; + describe("calculateStreaks", () => {Then remove the inline
formatDatedefinitions from each test.
12-24: Consider adding edge case tests for robustness.The test suite covers core scenarios well. For improved coverage, consider adding tests for:
- Entries with
count: 0(should not count as active days)- Unsorted input array (verifies internal sorting works correctly)
These are optional improvements that would increase confidence in edge case handling.
lib/db.ts (2)
165-168: Consider using explicit type instead ofany.The sorting callback uses
anyfor the parameters. UsingUserEntrywould provide better type safety.🔎 Proposed fix
- return data.entries.sort((a: any, b: any) => b.total_points - a.total_points); + return (data.entries as UserEntry[]).sort((a, b) => b.total_points - a.total_points);
214-237: Replaceanywith proper types for improved type safety.Multiple places use
anytype annotations. Consider usingUserEntryand explicit activity types for better maintainability.🔎 Proposed improvements
- const contributor = data.entries.find((entry: any) => entry.username.toLowerCase() === username.toLowerCase()); + const contributor = data.entries.find((entry: UserEntry) => entry.username.toLowerCase() === username.toLowerCase());For the activity mapping, consider defining a local type or using the raw_activities type from UserEntry:
- const activities = (contributor.activities || []).map((a: any) => ({ + const activities = (contributor.raw_activities || []).map((a) => ({ ...a, occured_at: new Date(a.occured_at), }));Note: Verify whether
contributor.activitiesorcontributor.raw_activitiesis the correct field based on theUserEntrytype definition.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/db.tsnext-env.d.tstests/lib/streak-utils.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
tests/lib/streak-utils.test.ts (1)
lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
lib/db.ts (2)
scripts/generateLeaderboard.ts (1)
UserEntry(83-98)lib/streak-utils.ts (2)
DailyActivity(1-5)calculateStreaks(10-95)
🔇 Additional comments (5)
next-env.d.ts (1)
1-6: Auto-generated file change looks correct.This path update from
.next/dev/types/routes.d.tsto.next/types/routes.d.tsaligns with Next.js 15+ conventions. Since the file comment indicates it should not be manually edited, ensure this change was generated by Next.js tooling (e.g.,next devornext build) rather than edited by hand.tests/lib/streak-utils.test.ts (1)
1-103: Good test coverage for streak utilities.The test suite validates key scenarios: empty input, consecutive days, gaps, and current streak logic based on today/yesterday. The use of dynamic dates for current streak tests is appropriate.
lib/db.ts (3)
278-296: Monthly activity bucket logic looks correct.The bucket ranges
[0-7),[7-14),[14-21),[21-28)correctly partition the month into weekly segments. The previous "1month" type mismatch has been resolved by using"month".
150-187: New utility functions are well-structured.The functions
getUpdatedTime,getLeaderboard,getAllContributorsWithAvatars,getAllContributorUsernames, andgetContributorfollow consistent patterns with proper null/empty checks and case-insensitive username matching.
196-216: Good use of unified default return object.The
defaultReturnconstant ensures consistent return shape across all early-exit paths, addressing the previous review feedback about inconsistent return types.
Introduced ActivityChart component using Recharts to visualize the distribution of contributor points. Integrated this chart into the ContributorDetail page to sit alongside monthly stats.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/people/ContributorDetail.tsx (1)
313-330: Fix the day conversion arithmetic.The arithmetic at line 329 (
Math.round(avgHours / 2.4) / 10) is incorrect for converting hours to days. This was flagged in a previous review but remains unaddressed.To convert hours to days, divide by 24:
avgHours / 24.The current formula produces inaccurate results. For example, with
avgHours = 50:
- Current:
Math.round(50 / 2.4) / 10 = 21 / 10 = 2.1 days- Correct:
50 / 24 ≈ 2.08 days🔎 Quick fix
- return avgHours > 24 ? `${Math.round(avgHours / 2.4) / 10} days` : `${avgHours}h`; + return avgHours > 24 ? `${(avgHours / 24).toFixed(1)} days` : `${avgHours}h`;As noted in the previous review, extracting this logic into a helper function (e.g.,
lib/pr-metrics.ts) would improve testability and readability.
🧹 Nitpick comments (1)
components/people/ActivityChart.tsx (1)
40-61: Replaceanytype with proper Recharts typing.The
CustomTooltipfunction usesanyfor its parameters, which bypasses TypeScript's type checking and reduces maintainability.🔎 Proposed type-safe implementation
+import type { TooltipProps } from "recharts"; + // Custom Tooltip - const CustomTooltip = ({ active, payload }: any) => { + const CustomTooltip = ({ active, payload }: TooltipProps<number, string>) => { if (active && payload && payload.length) { const data = payload[0].payload;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/people/ActivityChart.tsxcomponents/people/ContributorDetail.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/people/ActivityChart.tsx (2)
components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)components/ui/tooltip.tsx (1)
Tooltip(61-61)
🔇 Additional comments (6)
components/people/ActivityChart.tsx (1)
28-109: LGTM! Well-structured chart component.The component properly transforms data, provides a custom tooltip with detailed metrics, handles empty data gracefully, and uses responsive containers for flexible layouts. The color mapping and legend formatting enhance usability.
components/people/ContributorDetail.tsx (5)
27-29: LGTM! Clean integration of new components and streak fields.The imports bring in AchievementBadges, ActivityChart, and badge utilities, while the ContributorEntry interface is extended with optional streak fields, maintaining backward compatibility.
Also applies to: 57-58
173-173: Correct handling of empty activity breakdown.The
|| 0fallback properly handles the case wheresortedActivitiesis empty (Math.max with an empty spread returns -Infinity), ensuring maxPoints defaults to 0.
181-183: LGTM! Server-side streaks and badge integration.Moving streak calculation to the server improves consistency and performance. The badge retrieval and display name helpers cleanly prepare data for the UI.
Also applies to: 196-200
255-282: Well-designed streak metric cards.The Current Streak and Longest Streak cards provide clear visual feedback with appropriate icons and conditional animation. The grid layout effectively showcases key contributor metrics.
299-299: LGTM! Clean component integration.The AchievementBadges and ActivityChart components are well-integrated, and the monthly activity section provides comprehensive metrics with a clear, accessible layout.
Also applies to: 354-389
|
Refactored the streak calculation logic to handle calendar gaps and timezone inconsistencies as requested. Ready for review! |
…and optimized repo overview
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 (1)
lib/db.ts (1)
324-345:⚠️ Potential issue | 🟠 MajorUse a 60-day or all-time data source for previous-month activity counting.
The filter for
30 <= daysAgo < 60requires data spanning at least 60 days, butmonth.jsonis a rolling 30-day window. While it may contain some activities in that range (e.g., from the previous month's tail), it cannot reliably capture the full previous-month window—2month.jsoncontains 80 activities in the 30-59 day range compared to only 6 inmonth.json, demonstrating significant data loss. Use2month.jsonoryear.jsonas the data source.
♻️ Duplicate comments (1)
lib/db.ts (1)
183-193:⚠️ Potential issue | 🟠 MajorGuard the shared
year.jsonloader before sorting it.
getLeaderboard()is now the base path for several helpers, but it still trustsJSON.parse()and only checks truthiness before calling.sort(). One malformed or partially written artifact will reject those requests instead of falling back cleanly. Please wrap the read/parse intry/catch, requireArray.isArray(data.entries), and reuse that safe loader ingetContributorProfile().As per coding guidelines, "Use try-catch or .catch() for all async function error handling" and "Validate user input and external data before processing".
🧹 Nitpick comments (2)
components/people/ContributorDetail.tsx (1)
51-68: Share the contributor response type.This is now the third local
ContributorEntrydefinition (app/api/people/route.tsandapp/people/page.tsxeach have another one), and the copies are already drifting around streak-related fields. Export one shared interface so API and UI changes stay aligned.scripts/generateLeaderboard.ts (1)
62-68: Reuse the shared streak helper instead of maintaining a second streak schema.This file now recalculates streaks locally and serializes them as
user.streak, whileapp/api/people/route.tsandlib/db.tsboth uselib/streak-utils.tsas the streak source of truth. Keeping another algorithm and another field shape here makes it easy for generated JSON to drift from the API. Import the shared helper here and derive onlylastActivityDatelocally if you still need it.Also applies to: 936-985
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c115f22b-2142-4c17-95a2-f70a17bb8359
📒 Files selected for processing (11)
app/api/people/route.tsapp/globals.csscomponents/people/ContributorDetail.tsxeslint.config.mjslib/db.tspublic/leaderboard/month.jsonpublic/leaderboard/overview.jsonpublic/leaderboard/recent-activities.jsonpublic/leaderboard/week.jsonpublic/leaderboard/year.jsonscripts/generateLeaderboard.ts
✅ Files skipped from review due to trivial changes (1)
- public/leaderboard/overview.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/globals.css
| if(combined.length < expectedCount){ | ||
| for(const [type, info] of Object.entries(entry.activity_breakdown)){ | ||
| const existingCount = combined.filter(a => a.type === type).length; | ||
| const missing = info.count - existingCount; | ||
| for(const [type, info] of Object.entries(entry.activity_breakdown)){ | ||
| const existingCount = combined.filter(a => a.type === type).length; | ||
| const missing = info.count - existingCount; | ||
|
|
||
| for(let i = 0; i < missing; i++){ | ||
| combined.push({ | ||
| type, | ||
| title: `${type} contribution`, | ||
| occured_at: new Date(0).toISOString(), | ||
| link: "", | ||
| points: info.count > 0 ? Math.round(info.points / info.count) : 0, | ||
| }); | ||
| for(let i = 0; i < missing; i++){ | ||
| combined.push({ | ||
| type, | ||
| title: `${type} contribution`, | ||
| occured_at: new Date(0).toISOString(), | ||
| link: "", | ||
| points: info.count > 0 ? Math.round(info.points / info.count) : 0, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Cap synthetic activity padding to the visible limit.
year.json entries are emitted with raw_activities, not activities, so when a yearly record is merged after a smaller period record missing can jump to the contributor’s full yearly total. This loop then allocates one epoch placeholder per missing contribution even though Line 116 immediately trims the array back to 15 items. Limit padding to the remaining display slots or skip synthetic fillers entirely.
💡 Suggested cap on filler generation
const expectedCount = Object.values(entry.activity_breakdown || {}).reduce((sum, v) => sum + v.count, 0);
- if(combined.length < expectedCount){
+ const maxVisibleActivities = 15;
+ const targetCount = Math.min(expectedCount, maxVisibleActivities);
+
+ if (combined.length < targetCount) {
for(const [type, info] of Object.entries(entry.activity_breakdown)){
const existingCount = combined.filter(a => a.type === type).length;
- const missing = info.count - existingCount;
+ const remainingSlots = Math.max(targetCount - combined.length, 0);
+ const missing = Math.min(
+ Math.max(info.count - existingCount, 0),
+ remainingSlots,
+ );
for(let i = 0; i < missing; i++){
combined.push({Also applies to: 116-116
| {/* Efficiency Metrics */} | ||
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> | ||
| <Card className="bg-gradient-to-br from-indigo-500/10 to-purple-500/10 border-indigo-500/20 shadow-sm"> | ||
| <CardHeader className="pb-2"> | ||
| <CardTitle className="text-sm font-medium flex items-center gap-2 text-indigo-600 dark:text-indigo-400"> | ||
| <Clock className="w-4 h-4" /> | ||
| PR Turn-around | ||
| </CardTitle> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <div className="flex items-baseline gap-2"> | ||
| <span className="text-3xl font-bold"> | ||
| {(() => { | ||
| const prOpened = new Map<string, number>(); | ||
| const times: number[] = []; | ||
| const sorted = [...(contributor.activities || [])].sort( | ||
| (a, b) => new Date(a.occured_at).getTime() - new Date(b.occured_at).getTime() | ||
| ); | ||
|
|
||
| sorted.forEach(act => { | ||
| if (act.type === "PR opened") prOpened.set(act.link, new Date(act.occured_at).getTime()); | ||
| else if (act.type === "PR merged" && prOpened.has(act.link)) { | ||
| times.push(new Date(act.occured_at).getTime() - prOpened.get(act.link)!); | ||
| } | ||
| }); | ||
|
|
||
| if (times.length === 0) return "N/A"; | ||
| const avgHours = Math.round(times.reduce((a, b) => a + b, 0) / times.length / (1000 * 3600)); | ||
| return avgHours > 24 ? `${Math.round(avgHours / 2.4) / 10} days` : `${avgHours}h`; | ||
| })()} | ||
| </span> | ||
| <span className="text-muted-foreground text-sm">avg. merge time</span> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyDays}</div> | ||
| <div className="text-sm text-muted-foreground">Active Days</div> | ||
| </CardContent> | ||
| </Card> | ||
|
|
||
| <Card className="bg-gradient-to-br from-orange-500/10 to-red-500/10 border-orange-500/20 shadow-sm"> | ||
| <CardHeader className="pb-2"> | ||
| <CardTitle className="text-sm font-medium flex items-center gap-2 text-orange-600 dark:text-orange-400"> | ||
| <TrendingUp className="w-4 h-4" /> | ||
| Activity Impact | ||
| </CardTitle> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <div className="flex items-baseline gap-2"> | ||
| <span className="text-3xl font-bold">{currentStreak}d</span> | ||
| <span className="text-muted-foreground text-sm">current streak</span> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyDays > 0 ? Math.round(monthlyPoints / monthlyDays) : 0}</div> | ||
| <div className="text-sm text-muted-foreground">Daily Average</div> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyActivityTypesCount}</div> | ||
| <div className="text-sm text-muted-foreground">Activity Types</div> | ||
| </div> | ||
| </div> | ||
| </CardContent> | ||
| </Card> | ||
| </CardContent> | ||
| </Card> | ||
| </div> | ||
|
|
||
| <div className="grid grid-cols-1 md:grid-cols-3 gap-6"> | ||
| <div className="md:col-span-2"> | ||
| <Card className="bg-gradient-to-r from-primary/5 to-primary/10 border-primary/20 h-full"> | ||
| <CardHeader> | ||
| <CardTitle className="flex items-center gap-2 text-primary"> | ||
| <BarChart3 className="w-5 h-5" /> | ||
| This Month's Activity | ||
| </CardTitle> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <div className="grid grid-cols-2 lg:grid-cols-3 gap-4"> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyPoints}</div> | ||
| <div className="text-sm text-muted-foreground">Points Earned</div> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyDays}</div> | ||
| <div className="text-sm text-muted-foreground">Active Days</div> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyDays > 0 ? Math.round(monthlyPoints / monthlyDays) : 0}</div> | ||
| <div className="text-sm text-muted-foreground">Daily Average</div> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg"> | ||
| <div className="text-2xl font-bold text-primary">{monthlyActivityTypesCount}</div> | ||
| <div className="text-sm text-muted-foreground">Activity Types</div> | ||
| </div> | ||
| <div className="text-center p-4 bg-background/50 rounded-lg lg:col-span-2"> | ||
| <div className="text-2xl font-bold text-primary">{(contributor.activities || []).length}</div> | ||
| <div className="text-sm text-muted-foreground">Total Actions Recorded</div> | ||
| </div> |
There was a problem hiding this comment.
These cards are reading aggregates from a display-only slice.
Line 116 in app/api/people/route.ts caps activities at 15, but Line 368 computes PR turn-around from that array and Line 436 labels its length as “Total Actions Recorded”. For active contributors, the count card can never exceed 15 and the turn-around card can miss the matching PR opened event entirely. Please render server-computed aggregates here instead of deriving them from the trimmed activity list.
| { | ||
| "type": "PR opened", | ||
| "occured_at": "2026-03-11T03:17:32Z", | ||
| "title": "Fix windows lint script", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/pull/1024", | ||
| "points": 2 | ||
| }, | ||
| { | ||
| "type": "PR opened", | ||
| "occured_at": "2026-03-10T11:51:51Z", | ||
| "title": "Fix v1 simulator base redirects", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/pull/1019", | ||
| "points": 2 | ||
| }, | ||
| { | ||
| "type": "Issue opened", | ||
| "occured_at": "2026-03-10T18:59:50Z", | ||
| "title": "🐞 Bug - Windows - npm run lint fails in PowerShell due to xargs usage", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/issues/1020", | ||
| "points": 1 | ||
| }, | ||
| { | ||
| "type": "Issue opened", | ||
| "occured_at": "2026-03-10T00:54:43Z", | ||
| "title": "🐞 Bug - Custom mount path breaks v1 redirect logic", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/issues/1017", | ||
| "points": 1 | ||
| }, | ||
| { | ||
| "type": "Issue labeled", | ||
| "occured_at": "2026-03-10T18:59:52Z", | ||
| "title": "Labeled issue #1020 - 🐞 bug", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/issues/1020", | ||
| "points": 2 | ||
| }, | ||
| { | ||
| "type": "PR opened", | ||
| "occured_at": "2026-03-09T18:24:57Z", | ||
| "title": "Make simulator API base configurable", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/pull/1016", | ||
| "points": 2 | ||
| }, | ||
| { | ||
| "type": "Issue opened", | ||
| "occured_at": "2026-03-09T17:36:45Z", | ||
| "title": "🐞 Bug - Route-agnostic mounting breaks API requests (hard-coded /api/...)", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/issues/1015", | ||
| "points": 1 | ||
| }, | ||
| { | ||
| "type": "Issue labeled", | ||
| "occured_at": "2026-03-10T00:54:44Z", | ||
| "title": "Labeled issue #1017 - 🐞 bug", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/issues/1017", | ||
| "points": 2 | ||
| }, | ||
| { | ||
| "type": "Issue labeled", | ||
| "occured_at": "2026-03-09T17:36:46Z", | ||
| "title": "Labeled issue #1015 - 🐞 bug", | ||
| "link": "https://github.com/CircuitVerse/cv-frontend-vue/issues/1015", | ||
| "points": 2 | ||
| } | ||
| ] |
There was a problem hiding this comment.
Keep activities sorted newest-first.
These new timelines are out of order. For mundele2004, Line 5541 is newer than the March 9 events starting at Line 5534 but appears after them. For h30s, Line 7379 is later than Line 7372 but is listed below it. If the UI renders activities as stored, the feed will jump backward in time. Sorting by occured_at descending before serializing this snapshot would avoid that.
Also applies to: 7369-7399
| { | ||
| "username": "Uday9909", | ||
| "name": null, | ||
| "avatar_url": "https://avatars.githubusercontent.com/u/178378295?v=4", | ||
| "role": "Contributor", | ||
| "total_points": 3, | ||
| "activity_breakdown": { | ||
| "Issue opened": { | ||
| "count": 1, | ||
| "points": 1 | ||
| }, | ||
| "Issue labeled": { | ||
| "count": 1, | ||
| "points": 2 | ||
| } | ||
| }, | ||
| "daily_activity": [ | ||
| { | ||
| "date": "2026-03-06", | ||
| "count": 2, | ||
| "points": 3 | ||
| } | ||
| ], | ||
| "activities": [ | ||
| { | ||
| "type": "Issue opened", | ||
| "occured_at": "2026-03-06T03:38:01Z", | ||
| "title": "🐞 Bug - Grades are deleted when an assignment is reopened due to submission fork destruction", | ||
| "link": "https://github.com/CircuitVerse/CircuitVerse/issues/7128", | ||
| "points": 1 | ||
| }, | ||
| { | ||
| "type": "Issue labeled", | ||
| "occured_at": "2026-03-06T03:38:02Z", | ||
| "title": "Labeled issue #7128 - 🐞 bug", | ||
| "link": "https://github.com/CircuitVerse/CircuitVerse/issues/7128", | ||
| "points": 2 | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Uday9909's weekly total is missing a March 4 activity.
public/leaderboard/recent-activities.json Lines 2157-2163 still include Uday9909's PR #7117 worth 2 points on 2026-03-04, and this file's startDate is also 2026-03-04. Keeping only the March 6 issue flow undercounts the contributor and makes the generated artifacts disagree.
| async function fetchAll<T = any>(url: string): Promise<T[]> { | ||
| let page = 1; | ||
| const results: T[] = []; | ||
| while (true) { | ||
| let page = 1; | ||
| const results: T[] = []; | ||
| while (true) { | ||
| const join = url.includes("?") ? "&" : "?"; | ||
| const res = await fetch(`${url}${join}per_page=100&page=${page}`, { | ||
| headers: { | ||
| Authorization: `Bearer ${TOKEN}`, | ||
| Accept: "application/vnd.github+json", | ||
| }, | ||
| }); | ||
| const fullUrl = `${url}${join}per_page=100&page=${page}`; | ||
| const res = await fetch(fullUrl, { | ||
| headers: { | ||
| Authorization: `Bearer ${TOKEN}`, | ||
| Accept: "application/vnd.github+json", | ||
| }, | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| const text = await res.text().catch(() => ""); | ||
| console.error(`❌ Failed to fetch ${fullUrl}: ${res.status} ${text}`); | ||
| throw new Error(`GitHub API ${res.status}: ${text}`); | ||
| } | ||
| await smartSleep(res, 500); | ||
| const data: T[] = await res.json(); | ||
| results.push(...data); | ||
|
|
||
| if (data.length < 100) break; | ||
| page++; | ||
| } | ||
|
|
||
| return results; | ||
| } | ||
|
|
||
| // ------ Metric fetchers ------ | ||
|
|
||
| async function fetchIssuesCreated(repo: string, current_start: Date) { | ||
| 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, current_start: Date) { | ||
| console.log(" 🔎 Fetching current PRs opened..."); | ||
|
|
||
| let count = 0; | ||
| let page = 1; | ||
|
|
||
| while (true) { | ||
| const res = await fetch( | ||
| `${GITHUB_API}/repos/${ORG}/${repo}/pulls?state=all&sort=created&direction=desc&per_page=100&page=${page}`, | ||
| { headers: { Authorization: `Bearer ${TOKEN}`, Accept: "application/vnd.github+json" } } | ||
| ); | ||
|
|
||
| if (!res.ok) break; | ||
|
|
||
| await smartSleep(res, 500); | ||
|
|
||
| const prs = await res.json(); | ||
| if (!prs.length) break; | ||
| const data = (await res.json()) as any; | ||
|
|
||
| for (const pr of prs) { | ||
| if (new Date(pr.created_at) < current_start) return count; // Early exit | ||
| if (!isBotUser(pr.user)) count++; | ||
| if (!data) { | ||
| console.warn(`⚠️ No data received from ${fullUrl}`); | ||
| break; | ||
| } | ||
|
|
||
| if (prs.length < 100) break; | ||
| page++; | ||
| } | ||
|
|
||
| return count; | ||
| } | ||
|
|
||
| async function fetchPRsMerge(repo:string, current_start: Date, previous_start: Date, now: Date) { | ||
| console.log(" 🔎 Fetching PRs merged..."); | ||
| const prs = await fetchAll( | ||
| `${GITHUB_API}/repos/${ORG}/${repo}/pulls?state=closed&sort=updated&direction=desc` | ||
| ); | ||
| // Handle both direct array and search result object { items: [] } | ||
| const items = Array.isArray(data) ? data : (data.items || []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 620 scripts/generateLeaderboard.ts | tail -n +570Repository: CircuitVerse/community-dashboard
Length of output: 1502
Replace any with unknown and implement proper type narrowing in fetchAll.
The function uses T = any default and as any cast, bypassing type checking. Remove the any default (require explicit type specification), use unknown for the parsed response, and narrow the type before accessing properties.
Suggested implementation
+interface GitHubCollectionResponse<T> {
+ items?: T[];
+}
+
-async function fetchAll<T = any>(url: string): Promise<T[]> {
+async function fetchAll<T>(url: string): Promise<T[]> {
let page = 1;
const results: T[] = [];
while (true) {
@@
- const data = (await res.json()) as any;
+ const data: unknown = await res.json();
@@
- const items = Array.isArray(data) ? data : (data.items || []);
+ const items: T[] = Array.isArray(data)
+ ? (data as T[])
+ : typeof data === "object" &&
+ data !== null &&
+ Array.isArray((data as GitHubCollectionResponse<T>).items)
+ ? (data as GitHubCollectionResponse<T>).items!
+ : [];Coding guideline: **/*.{ts,tsx} files must avoid any type and use unknown for type safety.
🎯 Overview
This PR adds server-side contribution streak calculation and introduces tracking for the longest streak on contributor profiles.
✨ What’s Included
Backend
calculateStreakshelper that determines:Frontend
current_streakandlongest_streakdirectly from the API.🛠️ Technical Notes
DailyActivityandContributorEntryinterfaces.✅ Testing
🔗 References
cc: @Naman-Chhabra @Aboobacker-MK @supreeth-c
Summary by CodeRabbit
Release Notes