feat(cron): trending info calc via github actions cron#86
feat(cron): trending info calc via github actions cron#86ketankauntia wants to merge 1 commit intomasterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Trending Data Types & Loading lib/trending-types.ts |
Introduces TypeScript types (TrendingEntity, TrendingRange, TrendingSnapshot, and specialized item interfaces for each entity) and loadTrendingSnapshot() async function with archive-aware path resolution logic for daily, weekly, monthly, yearly snapshots. Includes validation helpers isValidTrendingEntity and isValidTrendingRange. |
Trending Page Structure app/trending/[entity]/layout.tsx, app/trending/[entity]/page.tsx |
New layout component wraps trending routes with Header/Footer. Page component handles dynamic routing, static param generation, metadata generation with SEO/OG tags, snapshot loading, and not-found fallbacks. Implements revalidation at 3600s. |
Trending Client Component app/trending/[entity]/trending-page-client.tsx |
Client-side component rendering entity navigation bar, time-range selector, snapshot metadata, and entity-specific trending items (grid layout for organizations, list layout for others) with rank badges and change indicators. Manages range/archive state and router navigation. |
Snapshot Generation Script scripts/generate-trending-data.ts |
New script querying Prisma database, calculating trending rankings and changes relative to previous snapshots, generating snapshots for all entities and ranges, writing atomically to both archive and latest files with ISO week support and deterministic naming. |
Navigation Integration components/header.tsx, components/footer-common.tsx, components/Footer.tsx |
Header adds "Trending" menu item pointing to /trending/organizations. Footer navigation restructured with new trending links (organizations, projects, tech-stack, topics) replacing deprecated GSoC yearly items; Community section adds Blog and Sitemap links. Footer grid changed from 3 to 2 columns. |
Sitemap & Build Config app/sitemap.ts, package.json |
Sitemap includes four trending routes with daily change frequency. Package.json adds generate:trending npm script and tsx dependency. |
GitHub Actions Workflow .github/workflows/generate-trending.yml |
New daily-scheduled workflow (00:00 UTC) that installs dependencies, generates Prisma Client, runs generate:trending script with DATABASE_URL secret, and commits updated snapshots using bot identity with [skip ci]. |
Architectural Documentation md-docs/12-trending-feature-architecture.md |
Comprehensive design document covering routing patterns, data storage layout, snapshot formats, generation workflow, archive system, UI component design, caching/static generation strategy, and GitHub Actions automation. |
Sequence Diagram(s)
sequenceDiagram
participant GHA as GitHub Actions
participant Script as generate-trending Script
participant DB as Prisma Database
participant FS as File System
participant Page as Page Component
participant Client as TrendingPageClient
participant Browser as Browser
GHA->>GHA: Scheduled daily at 00:00 UTC
GHA->>Script: Trigger: npm run generate:trending
Script->>DB: Query trending data<br/>(organizations, projects, etc.)
DB-->>Script: Return ranked items
Script->>Script: Calculate changes vs.<br/>previous snapshot
Script->>FS: Write snapshot atomically<br/>(latest + archive files)
FS-->>Script: Files persisted
Browser->>Page: Request /trending/[entity]?range=daily
Page->>FS: loadTrendingSnapshot(entity, range)
FS-->>Page: Return TrendingSnapshot JSON
Page->>Page: generateMetadata() +<br/>generateStaticParams()
Page->>Client: Pass snapshot + range
Client->>Client: Render entity nav,<br/>range selector, items
Client-->>Browser: HTML + interactive UI
Browser->>Browser: User selects range/archive
Browser->>Page: router.push(new query params)
Page->>FS: loadTrendingSnapshot<br/>(new params)
FS-->>Page: Return archive snapshot
Page->>Client: Updated snapshot
Client-->>Browser: Re-render with new data
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
- PR
#54: Both modifyFOOTER_NAVIGATION_ITEMSincomponents/footer-common.tsxfor footer navigation link restructuring. - PR
#68: Both updateFOOTER_NAVIGATION_ITEMSincomponents/footer-common.tsxwith changes to footer navigation hrefs. - PR
#84: Both modifycomponents/header.tsxto add new header navigation menu items (main PR adds "Trending", related PR adds "Projects" and "Yearly").
Poem
🐰 Trending snapshots, ranked with care,
Daily wisdom floating in the air,
Organizations, projects glowing bright,
Tech stacks and topics dancing in moonlight,
Zero runtime, pure static sight! ✨
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description is entirely incomplete, with all required sections left empty (Summary, Testing) and all checklist items unchecked. | Fill in the Summary section with details about the trending feature implementation, add Testing section describing verification steps, and check the appropriate checklist items before merging. | |
| Docstring Coverage | Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main change: adding automated trending information calculation via GitHub Actions cron scheduling. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/workflows/generate-trending.yml:
- Around line 18-27: The workflow sets up Node.js with cache: 'npm' while the
job installs pnpm (pnpm/action-setup@v4), causing a cache mismatch; update the
Setup Node.js step to use cache: 'pnpm' (or remove/align the cache setting) so
the package manager (pnpm) and the cache configuration match, targeting the
Setup Node.js step and the cache: 'npm' value in the diff.
In `@app/trending/`[entity]/page.tsx:
- Around line 148-163: The fallback UI inside the if (!snapshot) branch exposes
implementation details; replace the content in that conditional (the Heading and
Text JSX that references entity and validRange) with a user-friendly message
that omits "run the trending data generation script" and instead explains the
data is temporarily unavailable (e.g., "Trending data for {entity}
({validRange}) is currently unavailable. Please try again later or contact
support."). Keep the existing layout and use the same Heading and Text
components so styling is unchanged, and ensure any references to entity and
validRange remain for context.
In `@app/trending/`[entity]/trending-page-client.tsx:
- Around line 272-330: The organization trending card duplicates
OrganizationCard rendering; refactor by either (A) adapting OrganizationCard to
accept a polymorphic prop type (e.g., Organization | TrendingOrganizationItem)
and optional trending props (rank, change) so OrganizationCard can render the
extra badge when those props exist, or (B) create a small adapter that maps
TrendingOrganizationItem to the Organization shape and passes rank/change as
separate optional props to OrganizationCard; update the trending page to use
OrganizationCard (or the adapter) and reuse formatChange for the change display
so layout/logo/name/styling are centralized in OrganizationCard instead of
duplicated in the entity === "organizations" block.
In `@lib/trending-types.ts`:
- Around line 92-98: The current approximation that computes week =
Math.ceil((month * 30.44) / 7) (used before building filePath and weekStr for
range === 'weekly') yields non‑continuous, gappy week numbers and must be fixed:
either require an explicit week parameter (add a week argument and use that to
build weekStr and filePath) or replace the approximation with a proper ISO week
calculation (compute the ISO week number for a representative date such as the
first day of the given month using a helper like getISOWeek(year, month, 1) and
use that value for week/weekStr); update the code paths that reference range,
month, week, weekStr and filePath/baseDir accordingly so weekly archives map to
real ISO week numbers.
In `@scripts/generate-trending-data.ts`:
- Around line 416-431: getArchiveIdentifier currently returns only {year, month}
for daily ranges which causes daily snapshots to overwrite (monthly filenames
like YYYY-MM.json); modify getArchiveIdentifier to include day for the 'daily'
range (e.g., return { year, month, day }) by extracting now.getDate(), and then
update the archive filename generation logic that builds the output name (the
code that serializes the identifier into "YYYY-MM.json") to include the day when
present so daily files become "YYYY-MM-DD.json" while weekly/yearly remain
unchanged.
- Around line 191-227: The code sets currentValue = 1 for every project which
makes trending meaningless; replace that assignment in the projects.map block so
currentValue is derived from a real metric on the project (e.g.,
project.mentors_count || project.contributors_count || project.activity_score ||
0), ensure previousValue falls back to 0 not currentValue (previousMap.get(slug)
|| 0), then call calculateChange(currentValue, previousValue) as before and keep
the stable sorting/ranking logic; update the metadata or field names if you
introduce a new metric property so TrendingItem reflects the chosen metric.
🧹 Nitpick comments (14)
scripts/generate-trending-data.ts (1)
21-45: Consider importing shared types fromlib/trending-types.ts.The AI summary mentions that
lib/trending-types.tsdefinesTrendingEntity,TrendingRange,TrendingSnapshot, andTrendingItemtypes. Duplicating these definitions here could lead to drift between the generator and consumers.♻️ Suggested change
-type TrendingEntity = 'organizations' | 'projects' | 'tech-stack' | 'topics'; -type TrendingRange = 'daily' | 'weekly' | 'monthly' | 'yearly'; - -interface TrendingItem { - id: string; - slug: string; - name: string; - change: number; - change_percent: number; - current_value: number; - previous_value: number; - rank: number; - metadata?: Record<string, unknown>; -} - -interface TrendingSnapshot { - entity: TrendingEntity; - range: TrendingRange; - snapshot_at: string; - items: TrendingItem[]; - meta: { - version: number; - total_items: number; - }; -} +import type { + TrendingEntity, + TrendingRange, + TrendingSnapshot, + TrendingItem +} from '../lib/trending-types';md-docs/12-trending-feature-architecture.md (6)
40-40: Minor: Capitalize "GitHub" correctly.Static analysis flagged that "Github" should be spelled "GitHub" (capital H).
-- **GitHub Actions**: `.github/workflows/generate-trending.yml` - Automated daily generation +- **GitHub Actions**: `.github/workflows/generate-trending.yml` - Automated daily generation(The current text appears correct, but verify the actual content matches the capitalization.)
48-50: Add language specifier to fenced code blocks.Per markdownlint MD040, fenced code blocks should have a language specified for syntax highlighting and accessibility.
Proposed fix
-``` +```text /trending/:entity</details> --- `87-111`: **Add language specifier to file structure code block.** <details> <summary>Proposed fix</summary> ```diff -``` +```text new-api-details/trending/ ├── organizations/ ...
307-330: Add language specifier to UI layout diagram.Proposed fix
-``` +```text ┌─────────────────────────────────────────┐ │ Header (from layout) │ ...
465-486: Add language specifier to file structure code block.Proposed fix
-``` +```text app/trending/ ├── [entity]/ ...
604-607: Minor spelling: "5min" should be "5 min" or "5 minutes".Proposed fix
-- If generation time > 5min: Parallelize entity processing +- If generation time > 5 minutes: Parallelize entity processinglib/trending-types.ts (2)
112-123: Consider adding JSON schema validation for parsed snapshots.The parsed JSON is cast directly to
TrendingSnapshotwithout validation. If a malformed or incompatible JSON file exists, this could cause runtime errors when accessing expected properties.Consider adding basic structure validation:
Proposed enhancement
// Read and parse JSON file const fileContent = fs.readFileSync(filePath, 'utf-8'); const data = JSON.parse(fileContent); // Basic validation if (!data || typeof data !== 'object' || !Array.isArray(data.items)) { if (process.env.NODE_ENV === 'development') { console.warn(`[TRENDING] Invalid snapshot structure: ${filePath}`); } return null; } return data as TrendingSnapshot;
113-118: Synchronous file operations in async function.Using
fs.existsSyncandfs.readFileSyncin an async function blocks the event loop. While acceptable for ISR/build-time scenarios with small files, consider using async variants for better scalability:Proposed enhancement (optional)
import { promises as fsPromises } from 'fs'; // Check if file exists and read try { const fileContent = await fsPromises.readFile(filePath, 'utf-8'); const data = JSON.parse(fileContent) as TrendingSnapshot; return data; } catch (err) { if ((err as NodeJS.ErrnoException).code === 'ENOENT') { // File not found if (process.env.NODE_ENV === 'development') { console.warn(`[TRENDING] File not found: ${filePath}`); } return null; } throw err; }app/trending/[entity]/trending-page-client.tsx (4)
71-76: Memoize static arrays to avoid recreation on each render.The
yearsandmonthsarrays are recreated on every render. Since they're static, consider moving them outside the component or memoizing them.Proposed fix
+const YEARS = Array.from({ length: 10 }, (_, i) => new Date().getFullYear() - i); +const MONTHS = Array.from({ length: 12 }, (_, i) => i + 1); + export function TrendingPageClient({ entity, snapshot, currentRange, archiveYear, archiveMonth, }: TrendingPageClientProps) { const router = useRouter(); const [isPending, startTransition] = useTransition(); const [showArchive, setShowArchive] = useState(!!archiveYear); const isArchiveView = archiveYear !== undefined; - const currentYear = new Date().getFullYear(); - const years = Array.from({ length: 10 }, (_, i) => currentYear - i); - const months = Array.from({ length: 12 }, (_, i) => i + 1);
278-282: Remove redundantprefetch={true}and use design tokens for colors.
prefetch={true}is the default behavior forLinkin Next.js App Router.- Per coding guidelines, use existing color tokens instead of hardcoding
bg-white,border-gray-200, etc.Proposed fix
<Link key={item.id} href={`/organizations/${item.slug}`} - prefetch={true} - className="block bg-white border border-gray-200 rounded-xl p-5 hover:shadow-md hover:border-gray-300 transition-all w-full dark:bg-card dark:border-border dark:hover:border-gray-600" + className="block bg-card border border-border rounded-xl p-5 hover:shadow-md hover:border-accent transition-all w-full" >
286-286: Use design tokens for background colors.
bg-gray-100should use semantic color tokens likebg-mutedfor consistency with the design system.Proposed fix
-<div className="w-12 h-12 rounded-lg bg-gray-100 dark:bg-muted flex items-center justify-center overflow-hidden shrink-0"> +<div className="w-12 h-12 rounded-lg bg-muted flex items-center justify-center overflow-hidden shrink-0">
298-298: Use design token for text color.
text-gray-400should betext-muted-foregroundfor consistency.Proposed fix
-<span className="text-lg font-semibold text-gray-400 dark:text-muted-foreground"> +<span className="text-lg font-semibold text-muted-foreground">components/footer-common.tsx (1)
87-132: Consider removing or documenting the commented-out GSoC Yearly section.Large blocks of commented-out code can create confusion. If this section is intentionally removed, consider deleting it entirely. If it's temporarily disabled pending future work, add a brief comment explaining why it's preserved.
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: 'npm' | ||
|
|
||
| - name: Install pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 9 |
There was a problem hiding this comment.
Cache configuration mismatch: using npm cache with pnpm.
The workflow uses pnpm for package management but configures cache: 'npm' in setup-node. This means the cache won't be utilized effectively.
🔧 Proposed fix
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 20
- cache: 'npm'
+ cache: 'pnpm'📝 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.
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20 | |
| cache: 'npm' | |
| - name: Install pnpm | |
| uses: pnpm/action-setup@v4 | |
| with: | |
| version: 9 | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20 | |
| cache: 'pnpm' | |
| - name: Install pnpm | |
| uses: pnpm/action-setup@v4 | |
| with: | |
| version: 9 |
🤖 Prompt for AI Agents
In @.github/workflows/generate-trending.yml around lines 18 - 27, The workflow
sets up Node.js with cache: 'npm' while the job installs pnpm
(pnpm/action-setup@v4), causing a cache mismatch; update the Setup Node.js step
to use cache: 'pnpm' (or remove/align the cache setting) so the package manager
(pnpm) and the cache configuration match, targeting the Setup Node.js step and
the cache: 'npm' value in the diff.
| if (!snapshot) { | ||
| return ( | ||
| <div className="container mx-auto px-4 py-16 lg:py-24 max-w-4xl"> | ||
| <div className="text-center"> | ||
| <Heading variant="section" className="mb-4 text-2xl lg:text-3xl"> | ||
| Trending data not available | ||
| </Heading> | ||
| <Text className="mt-4 text-muted-foreground text-base max-w-md mx-auto"> | ||
| The trending snapshot for {entity} ({validRange}) has not been generated yet. | ||
| <br /> | ||
| <br /> | ||
| Please run the trending data generation script to populate this page. | ||
| </Text> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Improve user-facing error message.
The fallback UI mentions "run the trending data generation script," which exposes implementation details to end users. Consider a more user-friendly message.
Proposed fix
<Text className="mt-4 text-muted-foreground text-base max-w-md mx-auto">
- The trending snapshot for {entity} ({validRange}) has not been generated yet.
- <br />
- <br />
- Please run the trending data generation script to populate this page.
+ Trending data for {entity} is not yet available for the selected time range.
+ <br />
+ <br />
+ Please check back later or try a different time range.
</Text>📝 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.
| if (!snapshot) { | |
| return ( | |
| <div className="container mx-auto px-4 py-16 lg:py-24 max-w-4xl"> | |
| <div className="text-center"> | |
| <Heading variant="section" className="mb-4 text-2xl lg:text-3xl"> | |
| Trending data not available | |
| </Heading> | |
| <Text className="mt-4 text-muted-foreground text-base max-w-md mx-auto"> | |
| The trending snapshot for {entity} ({validRange}) has not been generated yet. | |
| <br /> | |
| <br /> | |
| Please run the trending data generation script to populate this page. | |
| </Text> | |
| </div> | |
| </div> | |
| ); | |
| if (!snapshot) { | |
| return ( | |
| <div className="container mx-auto px-4 py-16 lg:py-24 max-w-4xl"> | |
| <div className="text-center"> | |
| <Heading variant="section" className="mb-4 text-2xl lg:text-3xl"> | |
| Trending data not available | |
| </Heading> | |
| <Text className="mt-4 text-muted-foreground text-base max-w-md mx-auto"> | |
| Trending data for {entity} is not yet available for the selected time range. | |
| <br /> | |
| <br /> | |
| Please check back later or try a different time range. | |
| </Text> | |
| </div> | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In `@app/trending/`[entity]/page.tsx around lines 148 - 163, The fallback UI
inside the if (!snapshot) branch exposes implementation details; replace the
content in that conditional (the Heading and Text JSX that references entity and
validRange) with a user-friendly message that omits "run the trending data
generation script" and instead explains the data is temporarily unavailable
(e.g., "Trending data for {entity} ({validRange}) is currently unavailable.
Please try again later or contact support."). Keep the existing layout and use
the same Heading and Text components so styling is unchanged, and ensure any
references to entity and validRange remain for context.
| {entity === "organizations" && ( | ||
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6 max-w-6xl mx-auto"> | ||
| {(snapshot.items as TrendingOrganizationItem[]).map((item) => { | ||
| const logoUrl = item.metadata?.img_r2_url || item.metadata?.logo_r2_url || item.metadata?.image_url; | ||
|
|
||
| return ( | ||
| <Link | ||
| key={item.id} | ||
| href={`/organizations/${item.slug}`} | ||
| prefetch={true} | ||
| className="block bg-white border border-gray-200 rounded-xl p-5 hover:shadow-md hover:border-gray-300 transition-all w-full dark:bg-card dark:border-border dark:hover:border-gray-600" | ||
| > | ||
| {/* Header with Logo and Rank */} | ||
| <div className="flex items-start gap-4 mb-4"> | ||
| <div className="w-12 h-12 rounded-lg bg-gray-100 dark:bg-muted flex items-center justify-center overflow-hidden shrink-0"> | ||
| {logoUrl && typeof logoUrl === 'string' ? ( | ||
| <Image | ||
| src={logoUrl} | ||
| alt={`${item.name} logo`} | ||
| width={48} | ||
| height={48} | ||
| className="w-full h-full object-contain" | ||
| unoptimized={true} | ||
| loading="lazy" | ||
| /> | ||
| ) : ( | ||
| <span className="text-lg font-semibold text-gray-400 dark:text-muted-foreground"> | ||
| {item.name.charAt(0)} | ||
| </span> | ||
| )} | ||
| </div> | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="flex items-center justify-between gap-2 mb-1"> | ||
| <h3 className="font-semibold text-foreground text-base line-clamp-1"> | ||
| {item.name} | ||
| </h3> | ||
| <span className="flex-shrink-0 w-6 h-6 rounded-full bg-gradient-to-br from-teal-100 to-teal-50 dark:from-teal-900/30 dark:to-teal-800/20 flex items-center justify-center font-bold text-xs text-teal-700 dark:text-teal-300 border border-teal-200 dark:border-teal-700"> | ||
| {item.rank} | ||
| </span> | ||
| </div> | ||
| {/* Change and Percentage - Inside Card */} | ||
| <div className="flex items-center gap-2"> | ||
| <span | ||
| className={cn( | ||
| "font-semibold text-xs px-2 py-1 rounded-md", | ||
| item.change >= 0 | ||
| ? "text-green-700 dark:text-green-400 bg-green-50 dark:bg-green-950/30" | ||
| : "text-red-700 dark:text-red-400 bg-red-50 dark:bg-red-950/30" | ||
| )} | ||
| > | ||
| {formatChange(item.change, item.change_percent)} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </Link> | ||
| ); | ||
| })} | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for OrganizationCard component files
fd -t f "OrganizationCard"Repository: ketankauntia/gsoc-orgs
Length of output: 48
🏁 Script executed:
# Also search for references to OrganizationCard in the codebase
rg -t tsx -t ts "OrganizationCard" --max-count 20Repository: ketankauntia/gsoc-orgs
Length of output: 93
🏁 Script executed:
# Search for files containing "OrganizationCard" definition or import
rg "export.*OrganizationCard|from.*OrganizationCard" --max-count 20Repository: ketankauntia/gsoc-orgs
Length of output: 134
🏁 Script executed:
cat -n components/organization-card.tsxRepository: ketankauntia/gsoc-orgs
Length of output: 5156
🏁 Script executed:
# Check how OrganizationCard is currently used in the codebase
rg "OrganizationCard" -A 3 -B 1 --no-heading -t js -t jsxRepository: ketankauntia/gsoc-orgs
Length of output: 93
🏁 Script executed:
# Find TrendingOrganizationItem type definition
rg "TrendingOrganizationItem" -B 2 -A 5Repository: ketankauntia/gsoc-orgs
Length of output: 2425
🏁 Script executed:
# Check the imports in the trending page file to understand data structure
head -50 app/trending/\[entity\]/trending-page-client.tsxRepository: ketankauntia/gsoc-orgs
Length of output: 1535
🏁 Script executed:
cat -n lib/trending-types.tsRepository: ketankauntia/gsoc-orgs
Length of output: 5668
Consider whether a shared organization card abstraction is feasible for both regular and trending contexts.
The trending card for organizations duplicates base card rendering logic (grid layout, logo/name display, styling). While OrganizationCard exists with extensible variant support, it expects the Organization type, whereas the trending context uses TrendingOrganizationItem—a structurally different type with rank and change data. Rather than extending OrganizationCard, evaluate whether to:
- Create a shared card abstraction that accepts both data types via a discriminated union or adapter pattern
- Extend
OrganizationCardto work withTrendingOrganizationItemby accepting polymorphic item types and optional trending-specific props (rank?, change?) - Document why separate implementations are justified given the different display contexts and data availability
🤖 Prompt for AI Agents
In `@app/trending/`[entity]/trending-page-client.tsx around lines 272 - 330, The
organization trending card duplicates OrganizationCard rendering; refactor by
either (A) adapting OrganizationCard to accept a polymorphic prop type (e.g.,
Organization | TrendingOrganizationItem) and optional trending props (rank,
change) so OrganizationCard can render the extra badge when those props exist,
or (B) create a small adapter that maps TrendingOrganizationItem to the
Organization shape and passes rank/change as separate optional props to
OrganizationCard; update the trending page to use OrganizationCard (or the
adapter) and reuse formatChange for the change display so
layout/logo/name/styling are centralized in OrganizationCard instead of
duplicated in the entity === "organizations" block.
| } else if (range === 'weekly' && month !== undefined) { | ||
| // Weekly uses ISO week format: YYYY-Www.json | ||
| // Note: month parameter is used to approximate week (1-12 maps to weeks) | ||
| // For exact week lookup, would need week parameter, but month is acceptable for archive discovery | ||
| const week = Math.ceil((month * 30.44) / 7); // Approximate conversion | ||
| const weekStr = String(week).padStart(2, '0'); | ||
| filePath = path.join(baseDir, 'weekly', `${year}-W${weekStr}.json`); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ISO 8601 week numbering valid range weeks per year
💡 Result:
In ISO 8601 week numbering (week-date), the valid week range is 01–52, with 53 allowed only in some years, because an ISO week-numbering year has either 52 or 53 full weeks. [1][2]
A year has 53 weeks if it starts on a Thursday, or if it is a leap year that starts on a Wednesday; otherwise it has 52 weeks. [1]
A practical check: the week number of December 28 is always the last ISO week number of that ISO week-year (so it will be 52 or 53). [1][2]
Sources: [1] https://en.wikipedia.org/wiki/ISO_week_date, [2] https://en.wikipedia.org/wiki/ISO_8601, [3] https://help.tableau.com/current/pro/desktop/en-us/dates_calendar.htm
🏁 Script executed:
# Examine the full context of the trending-types.ts file around lines 92-98
cd /tmp/repo 2>/dev/null || cd .
fd -e ts -e tsx | grep -i trending | head -5Repository: ketankauntia/gsoc-orgs
Length of output: 241
🏁 Script executed:
# Calculate what the formula actually produces for months 1-12
python3 << 'EOF'
import math
print("Month to Week approximation (Math.ceil((month * 30.44) / 7)):")
print("Month | Calculated Week")
print("------|----------------")
for month in range(1, 13):
week = math.ceil((month * 30.44) / 7)
print(f" {month:2d} | {week:2d}")
print("\nValid ISO 8601 week range: 01-53 (some years have 53 weeks)")
EOFRepository: ketankauntia/gsoc-orgs
Length of output: 440
🏁 Script executed:
# Let me check the full context of the file and understand how this week calculation is used
cat lib/trending-types.ts 2>/dev/null || find . -name "trending-types.ts" -exec cat {} \;Repository: ketankauntia/gsoc-orgs
Length of output: 4604
🏁 Script executed:
# Also check if there are any tests or usage examples
fd -e test -e spec | xargs grep -l "trending" 2>/dev/null | head -5Repository: ketankauntia/gsoc-orgs
Length of output: 48
Week calculation approximation creates sparse, gappy week numbers that may miss data lookups.
The formula Math.ceil((month * 30.44) / 7) produces non-continuous week numbers: 5, 9, 14, 18, 22, 27, 31, 35, 40, 44, 48, 53. This means January (month 1) maps to week 5, skipping weeks 1–4 that actually exist in the calendar. Similarly, many weeks throughout the year (e.g., weeks 6–8, 10–13) will never have corresponding files, making this approximation problematic for archive discovery where users expect to find data for all calendar weeks.
While ISO 8601 does allow week 53 in certain years, the larger concern is the non-contiguous mapping. Consider either:
- Accepting this limitation and documenting which weeks are actually queryable, or
- Requiring explicit week numbers instead of month-based approximation for accurate lookups
🤖 Prompt for AI Agents
In `@lib/trending-types.ts` around lines 92 - 98, The current approximation that
computes week = Math.ceil((month * 30.44) / 7) (used before building filePath
and weekStr for range === 'weekly') yields non‑continuous, gappy week numbers
and must be fixed: either require an explicit week parameter (add a week
argument and use that to build weekStr and filePath) or replace the
approximation with a proper ISO week calculation (compute the ISO week number
for a representative date such as the first day of the given month using a
helper like getISOWeek(year, month, 1) and use that value for week/weekStr);
update the code paths that reference range, month, week, weekStr and
filePath/baseDir accordingly so weekly archives map to real ISO week numbers.
| const items: TrendingItem[] = projects | ||
| .map((project) => { | ||
| const slug = project.project_id; | ||
| const currentValue = 1; | ||
| const previousValue = previousMap.get(slug) || currentValue; | ||
| const { change, change_percent } = calculateChange(currentValue, previousValue); | ||
|
|
||
| return { | ||
| id: project.project_id, | ||
| slug, | ||
| name: project.project_title || project.project_id, | ||
| change, | ||
| change_percent, | ||
| current_value: currentValue, | ||
| previous_value: previousValue, | ||
| rank: 0, | ||
| metadata: { | ||
| org_slug: project.org_slug, | ||
| org_name: project.org_name, | ||
| year: project.year, | ||
| }, | ||
| }; | ||
| }) | ||
| .sort((a, b) => { | ||
| // Stable ranking: by current_value desc, then by id for tie-breaker | ||
| if (b.current_value !== a.current_value) { | ||
| return b.current_value - a.current_value; | ||
| } | ||
| return a.id.localeCompare(b.id); | ||
| }) | ||
| .map((item, index) => ({ | ||
| ...item, | ||
| rank: index + 1, | ||
| })); | ||
|
|
||
| return items; | ||
| } |
There was a problem hiding this comment.
Projects trending metric is effectively static.
Setting currentValue = 1 for all projects (Line 194) means every project has the same value. After the first run, change and change_percent will always be 0 since previousValue defaults to currentValue when not found. This makes "trending projects" a misnomer—it's just a list of recent projects sorted by year.
Consider using a meaningful metric such as:
- Number of mentors/contributors
- Related project count per organization
- Activity indicators if available
🤖 Prompt for AI Agents
In `@scripts/generate-trending-data.ts` around lines 191 - 227, The code sets
currentValue = 1 for every project which makes trending meaningless; replace
that assignment in the projects.map block so currentValue is derived from a real
metric on the project (e.g., project.mentors_count || project.contributors_count
|| project.activity_score || 0), ensure previousValue falls back to 0 not
currentValue (previousMap.get(slug) || 0), then call
calculateChange(currentValue, previousValue) as before and keep the stable
sorting/ranking logic; update the metadata or field names if you introduce a new
metric property so TrendingItem reflects the chosen metric.
| function getArchiveIdentifier(range: TrendingRange): { year: number; month?: number; week?: number } { | ||
| const now = new Date(); | ||
| const year = now.getFullYear(); | ||
|
|
||
| if (range === 'yearly') { | ||
| return { year }; | ||
| } | ||
|
|
||
| if (range === 'weekly') { | ||
| const { year: isoYear, week } = getISOWeek(now); | ||
| return { year: isoYear, week }; | ||
| } | ||
|
|
||
| const month = now.getMonth() + 1; | ||
| return { year, month }; | ||
| } |
There was a problem hiding this comment.
Daily archive files will overwrite within the same month.
getArchiveIdentifier returns only { year, month } for daily ranges, but the archive filename logic (Line 482) produces YYYY-MM.json. This means daily snapshots will overwrite each other throughout the month instead of creating daily archives (YYYY-MM-DD.json).
🐛 Proposed fix
function getArchiveIdentifier(range: TrendingRange): { year: number; month?: number; week?: number; day?: number } {
const now = new Date();
const year = now.getFullYear();
if (range === 'yearly') {
return { year };
}
if (range === 'weekly') {
const { year: isoYear, week } = getISOWeek(now);
return { year: isoYear, week };
}
const month = now.getMonth() + 1;
+
+ if (range === 'daily') {
+ const day = now.getDate();
+ return { year, month, day };
+ }
+
return { year, month };
}And update the filename generation:
} else {
- // Daily/Monthly: YYYY-MM or YYYY-MM-DD
- archiveFileName = `${archiveId.year}-${String(archiveId.month).padStart(2, '0')}.json`;
+ // Monthly: YYYY-MM, Daily: YYYY-MM-DD
+ if (range === 'daily' && archiveId.day) {
+ archiveFileName = `${archiveId.year}-${String(archiveId.month).padStart(2, '0')}-${String(archiveId.day).padStart(2, '0')}.json`;
+ } else {
+ archiveFileName = `${archiveId.year}-${String(archiveId.month).padStart(2, '0')}.json`;
+ }
}Also applies to: 474-483
🤖 Prompt for AI Agents
In `@scripts/generate-trending-data.ts` around lines 416 - 431,
getArchiveIdentifier currently returns only {year, month} for daily ranges which
causes daily snapshots to overwrite (monthly filenames like YYYY-MM.json);
modify getArchiveIdentifier to include day for the 'daily' range (e.g., return {
year, month, day }) by extracting now.getDate(), and then update the archive
filename generation logic that builds the output name (the code that serializes
the identifier into "YYYY-MM.json") to include the day when present so daily
files become "YYYY-MM-DD.json" while weekly/yearly remain unchanged.
1 participant
Summary
Testing
Checklist
Summary by CodeRabbit
New Features
Style
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.