Repository navigation
feat(code-review): Team tab with a per-person scorecard for reviews and CI - #2018
Conversation
…nd CI One row per GitHub login from PR reviews and CI runs: merged, time to merge, fix rate at merge, critical findings per PR, first-try green, reruns, shipped clean and runner time, shown next to the team median. Ranked is opt-in and ranks rates only, for people with at least five reviewed pull requests. Bots are left out, and CI runs with no job that ran (fork runs waiting for approval) are dropped, so they no longer read as hours of failed CI.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 6 minutes. View limit details
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. |
This comment has been minimized.
This comment has been minimized.
Makisuo
added
the
preview:web
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
🍁 Maple PR previewNote Preview resources were removed when this pull request closed. Final commit |
…default-branch list
Maple review🟢 Confidence 8/10 · likely safe to merge. Held at 8 because earlier reviews of this pull request found 1 defect. Adds the Code Review Team tab: a tenant-scoped
Fixed since the last review
|
Preview test (app-pr-2018.maple.dev,
|
| Check | Result |
|---|---|
/code-review/team inside the shell, Team tab current |
✅ "No team activity yet" on-ramp |
GET /_api/internal/code-review/team |
✅ 200, ~340ms round trip, expected shape |
| Analytics → CI → Team | ✅ Team re-renders from the retained answer, no second request |
Time range change (?timePreset=1w) |
✅ new request, 200 |
Unknown ?repo= |
✅ "Nobody in this window" + Clear filters, which returns to the default |
?ranked=true, ?sort=fixRate deep links |
✅ load cleanly |
| 390px mobile | ✅ no horizontal overflow (scrollWidth 390 = clientWidth) |
| Console / page errors from this page | ✅ none. The only errors were 503s from the live-sync shape stream, which preview:web doesn't deploy |
An invalid ?sort=bogus shows "Page not found". That is how the app treats bad search params on the other Code Review tabs too (CI with ?repo=not-a-uuid), so I left it.
Traces (deployment.environment = pr-2018)
11 CodeReviewTeamService.team spans, all Ok:
- 10–12ms when an unknown repository id short-circuits;
- 62–156ms for full reads, median about 90ms.
The slowest trace, 15bed8fb759281667deb6c3a2b774bff (156ms):
CodeReviewTeamService.team 156ms
├─ SELECT vcs_repositories 34ms (serial: scope + default branches)
├─ SELECT pr_reviews (per-PR fold) ~0ms ┐
├─ SELECT pr_review_findings 18ms │ in parallel
├─ executeSql ciActorStats 122ms │ 68ms of it: warehouse settings lookup
└─ executeSql ciQueueTotals 107ms ┘ (same lookup, run twice in parallel)
- No N+1. Two warehouse queries and three Postgres reads per request.
- The time goes to the warehouse settings lookup (68ms), which runs once per warehouse read. Two parallel reads both miss and hit Postgres. That duplicate already exists in
WarehouseQueryServiceand also shows on the CI tab, so I'm not fixing it here. - The 34ms repository read could overlap with the Postgres reads (only the warehouse queries need it). That would save about 30ms; not worth the extra plumbing for now.
- In the browser, the first
FetchCodeReviewTeamtook 1.38s on a cold page load. Every other startup request in parallel took about 1.3s too (customer, infra presence, integration statuses), so that is page-load contention, not this endpoint.
Ready to merge from my side once CI is green.
Adds a Team tab to Code Review (
/code-review/team): one row per person, built from the PR reviews and CI runs Maple already stores, with the team's health numbers above it.New page, so there is no before: the tab strip had Analytics, Pull requests, CI, Issues and Settings. The screenshots are
/lab/code-review-team, the real page inside the real shell over a seven-person fixture team (made-up logins, so the avatars are generated tiles).What it shows
?ranked=true): numbers people by one rate or median, only for people with at least 5 reviewed PRs. Count columns (merged, CI time) can't be ranked, so ranking never rewards volume.Making the CI data fair
Checked on our own org first. Two things would have made a naive leaderboard lie:
ciActorStatsQueryonly counts runs with a job that actually ran, and measures runner time from jobs minus queue: that contributor drops to 0.14h.[bot]logins) are left out of the people rows. They still count in team runner time and the default-branch rate, since they share the runners.How it works
GET /internal/code-review/team→CodeReviewTeamService. It reads per-PR folds ofpr_reviews(a PR counts when its first review is in the window), per-PR finding counts, the CI per-actor query and runner queue totals, all at once, then merges them by GitHub login.team/derive.ts.Verification
CodeReviewTeamService.test.ts(PGlite and a stubbed warehouse): the fold across a PR's reviews, a re-reviewed older PR not counting, fix rate judged at merge, bots dropped, CI-only people, per-repo default branch in the SQL, and another org's repo id reading nothing. Writing it caught a real bug:having min(created_at)loses the timestamp codec.derive.test.tscovers rates, medians, ordering with nulls last, ranking eligibility, shared ranks and highlights.team.scene.test.tscovers fetch on entry, sort and ranked URLs, the median row, ranked filtering, inert count headers and the empty on-ramp.src/ch+src/benchmark: 843 passed. Lint: no errors.Not covered here: human reviewers. The webhook doesn't ingest
pull_request_reviewevents yet, so approvals and time to first human review are a follow-up.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.