⚡ perf: deduplicate concurrent GitHub API requests#489
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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.
Code Review
This pull request introduces a request-deduplication mechanism for fetching GitHub user profiles in the OG image route using an inflightRequests map, and adds a corresponding integration test to verify concurrent request deduping. The feedback recommends adding explicit return types to the anonymous async function in the route handler and the mock fetch implementation in the test file to improve type safety.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const data = await res.json(); | ||
| let fetchPromise = inflightRequests.get(username); | ||
| if (!fetchPromise) { | ||
| fetchPromise = (async () => { |
There was a problem hiding this comment.
Add an explicit return type to the anonymous async function to ensure type safety and adhere to the TypeScript guidelines.
| fetchPromise = (async () => { | |
| fetchPromise = (async (): Promise<GitHubProfile | null> => { |
References
- Maintain explicit return types for functions in TypeScript to ensure type safety and API clarity.
| const mockFetch = vi.spyOn(global, "fetch").mockImplementation(() => { | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })); | ||
| }, 50); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The mock implementation returns a Promise but is not defined as an async function, and it lacks an explicit return type. To align with the repository's TypeScript guidelines, please update it to be an async function with an explicit return type.
| const mockFetch = vi.spyOn(global, "fetch").mockImplementation(() => { | |
| return new Promise((resolve) => { | |
| setTimeout(() => { | |
| resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })); | |
| }, 50); | |
| }); | |
| }); | |
| const mockFetch = vi.spyOn(global, "fetch").mockImplementation(async (): Promise<Response> => { | |
| return new Promise<Response>((resolve) => { | |
| setTimeout(() => { | |
| resolve(new Response(JSON.stringify({ name: "Valid User" }), { status: 200 })); | |
| }, 50); | |
| }); | |
| }); |
References
- In TypeScript, ensure functions and mock implementations have explicit return types and use async functions for mocks returning Promises to maintain type safety and readability.
| } | ||
| return null; |
There was a problem hiding this comment.
When the first in-flight GitHub request for a username returns a temporary non-OK response, this promise resolves to null and every concurrent caller renders the fallback OG image. Before this change, those callers fetched independently, so a later concurrent request could still return real profile data instead of sharing the first transient failure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/og/[username]/route.tsx
Line: 61-62
Comment:
**Shared Failure Result**
When the first in-flight GitHub request for a username returns a temporary non-OK response, this promise resolves to `null` and every concurrent caller renders the fallback OG image. Before this change, those callers fetched independently, so a later concurrent request could still return real profile data instead of sharing the first transient failure.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Superseded by #495, which changes the same OG route and tests with a completed 5/5 review. |
💡 What: Implemented an in-memory Map to cache in-flight promises.
🎯 Why: To prevent redundant identical fetches and avoid rate limits.
📊 Measured Improvement: Verified via unit tests that concurrent requests are deduplicated into a single fetch.
PR created automatically by Jules for task 6185246957405465171 started by @is0692vs
Greptile Summary
This PR adds in-flight deduplication for OG GitHub API requests.
Confidence Score: 4/5
The OG dedupe path needs a fix before merging.
src/app/api/og/[username]/route.tsx
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant R1 as Request 1 participant R2 as Request 2 participant M as inflightRequests participant GH as GitHub API R1->>M: lookup username M-->>R1: miss R1->>M: store fetch promise R1->>GH: fetch profile R2->>M: lookup username M-->>R2: same promise GH-->>R1: non-OK response R1->>M: delete username M-->>R1: null result M-->>R2: null result R1-->>R1: render fallback OG image R2-->>R2: render fallback OG image%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant R1 as Request 1 participant R2 as Request 2 participant M as inflightRequests participant GH as GitHub API R1->>M: lookup username M-->>R1: miss R1->>M: store fetch promise R1->>GH: fetch profile R2->>M: lookup username M-->>R2: same promise GH-->>R1: non-OK response R1->>M: delete username M-->>R1: null result M-->>R2: null result R1-->>R1: render fallback OG image R2-->>R2: render fallback OG imagePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "perf: deduplicate concurrent GitHub API ..." | Re-trigger Greptile