feat: add maintainer analytics trends#195
Conversation
|
@saurabhhhcodes is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks again for assigning #92. I kept this scoped to the maintainer analytics page and moved the aggregation into Postgres so the UI only receives compact chart data. When reviewing, could you also add the appropriate GSSoC approval / difficulty / type labels if this fits the program scoring? I included the EXPLAIN ANALYZE query in the PR body for the performance check. |
|
Synced this branch with the latest Validation after the sync:
I also retried |
Siddhartha-singh01
left a comment
There was a problem hiding this comment.
The code itself is genuinely solid @saurabhhhcodes using a stable Postgres function
to do the aggregation and return JSONB sidesteps the "unbounded query / 1000-row
cap" issue we hit on #136 and #185, and your cache key correctly includes user.id
(avoiding the cross-user leak we caught on #182). Rate limit, maintainer check,
scope filter, and defensive RPC parsing are all in place. Good architecture.
Two things to sort before merge:
-
The conflicts on maintainer.ts and page.tsx. These came from #151 landing on
main after your last sync. Reminder of what to watch for: your earlier PR #151
had a merge corruption where exportPrQueueCsv was never closed before another
function started, and the file didn't parse. Whatever path you pick to resolve
(web editor, rebase locally, or Copilot), please verify by opening
maintainer.ts after the resolve and confirming everyexport async functionhas
its matching}, then runnpm run typechecklocally before pushing. -
Migration number coordination. You're on 0012_ which is currently free on
main. But PR #185 and PR #177 also want migration numbers around this range.
Whoever lands first gets 0012_; the others bump. If #185 merges before this,
you'll need to rename to 0013_ (and update the test if it references the
filename).
Once conflicts are resolved cleanly, CI is green on the new commit, and the
migration number is confirmed free, this is ready to merge. The feature design is
strong.
|
@saurabhhhcodes just resolve the conflicts |
Summary
/maintainerPerformance notes
The heavy aggregation is done in Postgres via
maintainer_analytics_trends(repo_names text[], as_of timestamptz default now()).EXPLAIN ANALYZEquery used for review:The query is bounded to 12 weeks for weekly PR/XP trends and 6 months for level distribution snapshots. It filters by
repo_full_name = any(repo_names)and uses existing indexed tables/columns aroundpull_requests.repo_full_name,pull_requests.state, recommendation issue joins, and profile/level-up lookups. The function returns one JSON payload, so the route avoids shipping raw event rows to React.Tests
npm run test -- src/lib/maintainer/analytics.test.tsnpm run lint -- --file 'src/app/(app)/maintainer/page.tsx' --file 'src/app/(app)/maintainer/analytics-trends.tsx' --file src/app/actions/maintainer.ts --file src/lib/maintainer/analytics.tsnpm run typechecknpm run buildpasses; existing unrelated warnings remain insrc/app/(app)/dashboard/sync-button.tsx,src/app/(app)/issues/issues-list.tsx, and metadataBase.Closes #92