⚡ Optimize fetchActivity promise loop by is0692vs · Pull Request #549 · Hiroki-org/github-user-summary

is0692vs · 2026-08-07T06:48:51Z

💡 What: Replaced the sequential for loop over the array of fetch promises in fetchActivity with await Promise.allSettled(promises).

🎯 Why: The previous implementation sequentially awaited each promise inside the for (const p of promises) loop. While the requests were initiated concurrently, awaiting them sequentially blocks execution from progressing through the events of early-resolving requests. This optimization parallelizes the awaits to eliminate this waiting bottleneck while explicitly preserving the loop's error handling semantics.

📊 Measured Improvement: In a standalone benchmark script, replacing sequential await over 3 promises with a concurrent Promise.allSettled pattern reduced execution time for 10,000 iterations from ~162.5ms to ~87.6ms, representing an ~46% improvement in the asynchronous synchronization overhead. Promise.allSettled is used (rather than Promise.all) to ensure strict functional equivalence with the original try/catch loop: if a subsequent promise rejects, it does not fail the entire operation prematurely; the loop processes fulfilled results and handles specific rejections (UserNotFoundError, RateLimitError) on an iteration-by-iteration basis exactly as the original code did.

PR created automatically by Jules for task 14797105358996802760 started by @is0692vs

Greptile Summary

Promise の結果を順番に await する実装を Promise.allSettled に置き換え、取得結果の順序とエラー種別ごとの処理を維持しています。ただし、早期終了を判断する前に不要な後続ページまで待つため、低速または停止したリクエストが全体をブロックします。

3ページ分の GitHub activity リクエストを Promise.allSettled で待機
fulfilled/rejected の状態に応じてイベント集約とエラー処理を実施
短いページを検出した際の集約終了条件は維持

Confidence Score: 4/5

不要な後続ページが遅延・停止するとプロフィールページと summary API 全体がブロックされるため、マージ前に早期終了できる待機方法へ修正が必要です。

Promise.allSettled が全ページの完了を要求する一方、各 fetch にタイムアウトがないため、1ページ目だけで終了可能な場合でも後続リクエストの遅延が利用者向け応答へ直接伝播します。

Files Needing Attention: src/lib/github.ts

Important Files Changed

Filename	Overview
src/lib/github.ts	fetchActivity の同期処理を allSettled に変更しているが、早期終了前にも全ページの完了を待つ遅延回帰がある。

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["3ページを同時に取得"] --> B["Promise.allSettled"]
  B --> C{"3件すべて完了したか"}
  C -- "いいえ" --> B
  C -- "はい" --> D["1ページ目から結果を処理"]
  D --> E{"100件未満またはエラー"}
  E -- "はい" --> F["終了またはエラー伝播"]
  E -- "いいえ" --> G["次ページを処理"]

Prompt To Fix All With AI

### Issue 1
src/lib/github.ts:688
**早期終了前に全ページ待機**

1ページ目が100件未満または即座にエラーとなり、不要な2・3ページ目のリクエストが遅延・停止した場合、`Promise.allSettled` は全ページが完了するまで結果処理を開始しないため、プロフィールページ全体の描画とダッシュボード summary API の応答もブロックされます。

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "⚡ Optimize fetchActivity loop to use Pro..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

Knowledge Base — GitHub Client

Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>

vercel · 2026-08-07T06:48:53Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
github-user-summary	Ignored		Aug 7, 2026 6:48am

google-labs-jules · 2026-08-07T06:48:53Z

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

coderabbitai · 2026-08-07T06:49:01Z

Warning

Review limit reached

@is0692vs, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6d894e28-703d-4c49-9dd0-2f8eaf1fd92e

📥 Commits

Reviewing files that changed from the base of the PR and between eb95c48 and 7ab9158.

📒 Files selected for processing (1)

src/lib/github.ts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

qodo-code-review · 2026-08-07T06:50:27Z

PR Summary by Qodo

Optimize fetchActivity await loop using Promise.allSettled

✨ Enhancement 🕐 10-20 Minutes

AI Description

• Replace sequential awaiting of page fetch promises with Promise.allSettled.
• Preserve per-page error handling for UserNotFoundError and RateLimitError.
• Reduce async synchronization overhead when processing multiple pages.

Diagram

graph TD
  A["fetchActivity()"] --> B["Create page promises"] --> C["Promise.allSettled(promises)"] --> D["Process results (break/throw)"] --> E["Aggregate heatmap & counts"] --> F["Return ActivityData"]
  B --> G["restGet()"] --> H{{"GitHub REST API"}}

High-Level Assessment

The following are alternative approaches to this PR:

1. Two-phase await (await page 1, then allSettled remaining if needed)

➕ Preserves early-return latency when page 1 has <100 events (no need to wait for other pages to settle).
➕ Still parallelizes the remaining awaits when additional pages are required.
➖ Slightly more complex control flow (split handling for first page vs remaining pages).
➖ Still includes a sequential await for the first page.

2. Wrap each promise to normalize errors and use Promise.all

➕ Keeps concurrency while avoiding allSettled-specific typing/branches.
➕ Can encode error-handling semantics explicitly per promise.
➖ More code and custom conventions; easy to drift from existing semantics.
➖ Less idiomatic than allSettled for mixed success/failure batching.

Recommendation: Current Promise.allSettled approach is reasonable for reducing per-promise await overhead when multiple pages are typically consumed. However, it changes the waiting behavior: the function now waits for all page requests to settle even if the first page indicates completion (<100 events). If end-to-end latency for low-activity users is important, prefer the two-phase await approach to keep the previous early-exit performance characteristics while still avoiding sequential awaits for pages 2..3.

Files changed (1) +6 / -4

Enhancement (1) +6 / -4

github.tsParallelize fetchActivity promise synchronization with Promise.allSettled +6/-4

Parallelize fetchActivity promise synchronization with Promise.allSettled

• Replaces the sequential 'for (const p of promises) await p' pattern with 'await Promise.allSettled(promises)' and iterates settled results. Keeps the existing behavior of accumulating events, breaking on a short page, and rethrowing UserNotFoundError/RateLimitError while breaking on other failures.

src/lib/github.ts

codecov · 2026-08-07T06:51:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

greptile-apps · 2026-08-07T06:51:18Z

-  for (const p of promises) {
-    try {
-      const events = await p;
+    const results = await Promise.allSettled(promises);


早期終了前に全ページ待機

1ページ目が100件未満または即座にエラーとなり、不要な2・3ページ目のリクエストが遅延・停止した場合、Promise.allSettled は全ページが完了するまで結果処理を開始しないため、プロフィールページ全体の描画とダッシュボード summary API の応答もブロックされます。

Knowledge Base Used: GitHub Client

Prompt To Fix With AI

This is a comment left during a code review. Path: src/lib/github.ts Line: 688 Comment: **早期終了前に全ページ待機** 1ページ目が100件未満または即座にエラーとなり、不要な2・3ページ目のリクエストが遅延・停止した場合、`Promise.allSettled` は全ページが完了するまで結果処理を開始しないため、プロフィールページ全体の描画とダッシュボード summary API の応答もブロックされます。 **Knowledge Base Used:** [GitHub Client](https://app.greptile.com/hiroki-org/-/custom-context/knowledge-base/hiroki-org/github-user-summary/-/docs/github-client.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

qodo-code-review · 2026-08-07T06:52:35Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

1. fetchActivity now blocks early-exit 📘 Rule violation ▣ Testability

Description

fetchActivity now awaits Promise.allSettled(promises) before iterating results, removing the
prior early-exit/fast-fail behavior where it could return or throw without waiting for later pages
to settle. This behavioral change can increase latency and timeout risk (including for
fetchUserSummary which awaits fetchActivity) and should be covered by updated/added tests per
the checklist.

Code

src/lib/github.ts[R688-691]

+    const results = await Promise.allSettled(promises);
+  for (const result of results) {
+    if (result.status === "fulfilled") {
+      const events = result.value;

Evidence

The cited PR change moves fetchActivity from awaiting page promises as it iterates (which allowed
breaking/throwing after page 1 when events.length < 100 or on an error path) to awaiting all page
promises up front via Promise.allSettled(promises), which necessarily delays reaching the
early-exit conditions until pages 2 and 3 have also settled. Because restGet(...) is invoked when
building promises (starting the underlying fetch(...) immediately), the new structure doesn’t
prevent requests from being initiated; it forces the caller to wait for completion of all pages
before returning/throwing, increasing latency in common “<100 events” and fast-fail scenarios and
impacting fetchUserSummary which awaits fetchActivity. Existing tests even note the prior
semantics (“Next promises aren't awaited”), but there’s no corresponding test update/addition in
this PR to assert the new behavior despite the observable change.

Rule 226120: Update or add tests when behavior changes
src/lib/github.ts[685-695]
src/lib/tests/github/fetchActivity.test.ts[148-161]
src/lib/github.ts[671-705]
src/lib/github.ts[143-149]
src/lib/github.ts[777-793]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`fetchActivity` now awaits `Promise.allSettled(promises)` before it evaluates early-exit/fast-fail conditions, which changes observable behavior: it can no longer return/throw after page 1 without waiting for pages 2 and 3 to settle. This can significantly increase response latency and timeout risk for typical users with `<100` events and for error paths, and per compliance a behavior change requires updating/adding tests to reflect and validate the new semantics.

## Issue Context
- The function creates `promises` via `pages.map(() => restGet(...))`, which starts the underlying `fetch(...)` immediately.
- Previously, the loop could stop awaiting subsequent page promises once page 1 indicated completion (`events.length < 100`) or when an error branch broke/threw, allowing earlier return/throw even though later requests may still be in-flight.
- The current `Promise.allSettled(promises)` happens before the loop, so even if page 1 is sufficient or should fail fast, the function won’t return/throw until later pages have settled.
- There is an existing test describing the prior early-break semantics (“Next promises aren't awaited”), but the implementation now contradicts that expectation.
- `fetchActivity` is used by `fetchUserSummary`, so any added latency directly slows user summary generation.

## Fix Focus Areas
- src/lib/github.ts[675-705]
- src/lib/github.ts[688-695]
- src/lib/github.ts[143-149]
- src/lib/__tests__/github/fetchActivity.test.ts[148-161]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Redundant rejection suppression 🐞 Bug ⚙ Maintainability

Description

With the new Promise.allSettled(promises) await, all page promises are awaited to completion, so
the “suppress unhandled promise rejections if we break early or throw” rationale no longer applies.
This makes the nearby suppression comment misleading and keeps unnecessary rejection handlers in a
path that now always awaits all promises anyway.

Code

src/lib/github.ts[688]

+    const results = await Promise.allSettled(promises);

Evidence

The function now awaits Promise.allSettled(promises) before it enters the loop that can break,
so it cannot exit before promises settle; therefore, the earlier unhandled-rejection suppression
rationale is no longer accurate under the new control flow.

src/lib/github.ts[685-694]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`fetchActivity` now awaits `Promise.allSettled(promises)` before any early break/throw logic can run. This means the code path can no longer "break early" before promises have settled, so the unhandled-rejection suppression comment/handlers are no longer aligned with actual behavior.

## Issue Context
The suppression loop was originally justified because the function could return/throw without awaiting later promises. After the `Promise.allSettled` change, that situation no longer occurs.

## Fix Focus Areas
- src/lib/github.ts[685-689]

### Suggested fix approach
Either:
- Remove the per-promise `catch` suppression entirely (since `allSettled` handles rejections), or
- Update the comment to reflect the new behavior and ensure logging semantics are intentional.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context used

✅ Compliance rules (platform): 30 rules

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

qodo-code-review · 2026-08-07T06:52:38Z

+    const results = await Promise.allSettled(promises);
+  for (const result of results) {
+    if (result.status === "fulfilled") {
+      const events = result.value;


1. fetchactivity now blocks early-exit 📘 Rule violation ▣ Testability

fetchActivity now awaits Promise.allSettled(promises) before iterating results, removing the prior early-exit/fast-fail behavior where it could return or throw without waiting for later pages to settle. This behavioral change can increase latency and timeout risk (including for fetchUserSummary which awaits fetchActivity) and should be covered by updated/added tests per the checklist.

Agent Prompt

## Issue description `fetchActivity` now awaits `Promise.allSettled(promises)` before it evaluates early-exit/fast-fail conditions, which changes observable behavior: it can no longer return/throw after page 1 without waiting for pages 2 and 3 to settle. This can significantly increase response latency and timeout risk for typical users with `<100` events and for error paths, and per compliance a behavior change requires updating/adding tests to reflect and validate the new semantics. ## Issue Context - The function creates `promises` via `pages.map(() => restGet(...))`, which starts the underlying `fetch(...)` immediately. - Previously, the loop could stop awaiting subsequent page promises once page 1 indicated completion (`events.length < 100`) or when an error branch broke/threw, allowing earlier return/throw even though later requests may still be in-flight. - The current `Promise.allSettled(promises)` happens before the loop, so even if page 1 is sufficient or should fail fast, the function won’t return/throw until later pages have settled. - There is an existing test describing the prior early-break semantics (“Next promises aren't awaited”), but the implementation now contradicts that expectation. - `fetchActivity` is used by `fetchUserSummary`, so any added latency directly slows user summary generation. ## Fix Focus Areas - src/lib/github.ts[675-705] - src/lib/github.ts[688-695] - src/lib/github.ts[143-149] - src/lib/__tests__/github/fetchActivity.test.ts[148-161]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-08-07T06:52:38Z

-  for (const p of promises) {
-    try {
-      const events = await p;
+    const results = await Promise.allSettled(promises);


2. Redundant rejection suppression 🐞 Bug ⚙ Maintainability

With the new Promise.allSettled(promises) await, all page promises are awaited to completion, so the “suppress unhandled promise rejections if we break early or throw” rationale no longer applies. This makes the nearby suppression comment misleading and keeps unnecessary rejection handlers in a path that now always awaits all promises anyway.

Agent Prompt

## Issue description `fetchActivity` now awaits `Promise.allSettled(promises)` before any early break/throw logic can run. This means the code path can no longer "break early" before promises have settled, so the unhandled-rejection suppression comment/handlers are no longer aligned with actual behavior. ## Issue Context The suppression loop was originally justified because the function could return/throw without awaiting later promises. After the `Promise.allSettled` change, that situation no longer occurs. ## Fix Focus Areas - src/lib/github.ts[685-689] ### Suggested fix approach Either: - Remove the per-promise `catch` suppression entirely (since `allSettled` handles rejections), or - Update the comment to reflect the new behavior and ensure logging semantics are intentional.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

⚡ Optimize fetchActivity loop to use Promise.allSettled

7ab9158

Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>

pull-request-size Bot added the size/S label Aug 7, 2026

dosubot Bot added the enhancement New feature or request label Aug 7, 2026

greptile-apps Bot reviewed Aug 7, 2026

View reviewed changes

qodo-code-review Bot reviewed Aug 7, 2026

View reviewed changes

is0692vs mentioned this pull request Aug 9, 2026

⚡ Optimize loop awaiting in fetchActivity #552

Closed

Navigation Menu

Uh oh!

⚡ Optimize fetchActivity promise loop - #549

⚡ Optimize fetchActivity promise loop#549
is0692vs wants to merge 1 commit into
mainfrom
perf-optimize-fetch-activity-14797105358996802760

is0692vs commented Aug 7, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

vercel Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

google-labs-jules Bot commented Aug 7, 2026

Uh oh!

coderabbitai Bot commented Aug 7, 2026

Review limit reached

Uh oh!

qodo-code-review Bot commented Aug 7, 2026

Uh oh!

codecov Bot commented Aug 7, 2026

Uh oh!

greptile-apps Bot Aug 7, 2026

Uh oh!

qodo-code-review Bot commented Aug 7, 2026

Uh oh!

qodo-code-review Bot Aug 7, 2026

Uh oh!

qodo-code-review Bot Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

is0692vs commented Aug 7, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

vercel Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-labs-jules Bot commented Aug 7, 2026

Uh oh!

coderabbitai Bot commented Aug 7, 2026

Review limit reached

Uh oh!

qodo-code-review Bot commented Aug 7, 2026

PR Summary by Qodo

Uh oh!

codecov Bot commented Aug 7, 2026

Codecov Report

Uh oh!

greptile-apps Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Aug 7, 2026

Code Review by Qodo

Uh oh!

qodo-code-review Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

is0692vs commented Aug 7, 2026 •

edited by greptile-apps Bot

Loading

vercel Bot commented Aug 7, 2026 •

edited

Loading