feat: support repo issue and pull count#149
Conversation
|
@bluwy is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds aggregated issue+PR counts to repository listings and the single-repo endpoint, and introduces two new routes to fetch open issue and open PR counts by querying GitHub's search API. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Server as API Server (route handler)
participant GitHub as GitHub Search API
Client->>Server: GET /repos/{owner}/{repo}/issue-count
Server->>GitHub: GET /search/issues?q=repo:owner/repo+state:open&per_page=1
GitHub-->>Server: { total_count: N, ... }
Server-->>Client: { count: N }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
routes/repos/[owner]/[repo]/issue-count.ts (1)
22-24: Consider short-TTL caching for this endpoint.This route spends Search API quota on every request. A small per-repo cache (e.g., 30–120s) would reduce throttling risk during traffic bursts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/repos/`[owner]/[repo]/issue-count.ts around lines 22 - 24, This route calls ghFetch for every request and should use a short per-repo TTL cache to avoid burning Search API quota; wrap the ghFetch call in a small in-memory cache (e.g., Map or LRU) keyed by `${event.context.params.owner}/${event.context.params.repo}`, store the fetched result (or the pending Promise) and a timestamp, return cached value when age < TTL (30–120s, e.g., 60s), and on cache-miss perform ghFetch and update the cache; update the code around the ghFetch call and the variable res to read/write the cache and consider storing the Promise to dedupe concurrent requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@types/index.ts`:
- Line 12: The GithubRepo type now requires issueAndPullCount but the producer
in routes/users/[name]/repos.ts still returns repos without it; update the
repo-building logic (the function constructing GithubRepo objects in that file)
to include an issueAndPullCount property for every returned object (compute it
from available issue/pr data if present, or set a sensible default like 0) so
the returned shape matches the GithubRepo interface and prevents runtime or type
errors.
---
Nitpick comments:
In `@routes/repos/`[owner]/[repo]/issue-count.ts:
- Around line 22-24: This route calls ghFetch for every request and should use a
short per-repo TTL cache to avoid burning Search API quota; wrap the ghFetch
call in a small in-memory cache (e.g., Map or LRU) keyed by
`${event.context.params.owner}/${event.context.params.repo}`, store the fetched
result (or the pending Promise) and a timestamp, return cached value when age <
TTL (30–120s, e.g., 60s), and on cache-miss perform ghFetch and update the
cache; update the code around the ghFetch call and the variable res to
read/write the cache and consider storing the Promise to dedupe concurrent
requests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ba5e562-b114-485c-b9a5-599597ea6ad8
📒 Files selected for processing (4)
routes/repos/[owner]/[repo]/index.tsroutes/repos/[owner]/[repo]/issue-count.tsroutes/repos/[owner]/[repo]/pull-count.tstypes/index.ts
closes #148
New endpoints:
/repos/[owner]/[repo]/issue-count=>{ count: number }/repos/[owner]/[repo]/pull-count=>{ count: number }Updated endpoints:
/repos/[owner]/[repo]returns newissueAndPullCountfield (combined open issues and PRs count returned by github)Thoughts needed
In #148 I mentioned using the Link header trick to retrieve the issue and pr count quickly, but it only works if the repo has more than 1 issues or prs for the header to appear (for pagination). So I went with the github search approach, however that has a stricter rate limit: 1800/hour instead of 5000/hour, however this is a separate rate limit bucket to the core apis.
Alternatively, we can use the graphql api to fetch the issue and pr count, which i calculate has a lower cost (and better rate limit) altogether (1 point per query), but at that point, maybe
/repos/[owner]/[repo]should just use the graphql api and fetch everything in one go?Any thoughts on how we should go from here? Or is the current implementation good enough?
Example graphql query test
Summary by CodeRabbit