⚡ Optimize GitHub fetchActivity early-exit concurrency#355
Conversation
…vity 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
More reviews will be available in 49 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily introduces code formatting and style consistency updates across src/lib/github.ts. It also implements a fast-fail mechanism in fetchActivity using Promise.race to immediately propagate critical errors, such as UserNotFoundError or RateLimitError, during concurrent page fetches. As there are no review comments provided, I have no feedback to offer.
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.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const pages = [1, 2, 3]; | ||
| const allEvents: GitHubEvent[] = []; | ||
|
|
||
| let criticalReject: (reason?: unknown) => void; |
There was a problem hiding this comment.
! を使って宣言することで、strict: true 下の TS2454 コンパイルエラーを回避できます。
| let criticalReject: (reason?: unknown) => void; | |
| let criticalReject!: (reason?: unknown) => void; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/github.ts
Line: 749
Comment:
確定代入アサーション `!` を使って宣言することで、`strict: true` 下の TS2454 コンパイルエラーを回避できます。
```suggestion
let criticalReject!: (reason?: unknown) => void;
```
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!
| // Suppress unhandled promise rejections for subsequent pages if we break early or throw | ||
| promises.forEach((p) => p.catch((e) => logger.error("Event fetch promise rejected:", e))); | ||
| promises.forEach((p) => | ||
| p.catch((e) => { | ||
| if (!(e instanceof UserNotFoundError) && !(e instanceof RateLimitError)) { | ||
| logger.error("Event fetch promise rejected:", e); | ||
| } | ||
| }), | ||
| ); |
There was a problem hiding this comment.
promises.forEach でアタッチされた .catch は、ループ内で await Promise.race([p, criticalErrorPromise]) が既にキャッチ処理中の「現在のページ」の p にも独立して発火します。非クリティカルエラーが現在処理中のページで起きた場合、ループの catch ブロックで break しつつ、forEach ハンドラー側でも logger.error が呼ばれる二重ログが発生します。機能的なバグではなく元のコードからの継続動作ですが、ループが既にエラーを処理済みにもかかわらずログが出るため混乱の元になります。
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/github.ts
Line: 771-778
Comment:
**ループで処理中のページのエラーも抑制ハンドラーでキャッチされる**
`promises.forEach` でアタッチされた `.catch` は、ループ内で `await Promise.race([p, criticalErrorPromise])` が既にキャッチ処理中の「現在のページ」の `p` にも独立して発火します。非クリティカルエラーが現在処理中のページで起きた場合、ループの `catch` ブロックで `break` しつつ、forEach ハンドラー側でも `logger.error` が呼ばれる二重ログが発生します。機能的なバグではなく元のコードからの継続動作ですが、ループが既にエラーを処理済みにもかかわらずログが出るため混乱の元になります。
How can I resolve this? If you propose a fix, please make it concise.1 participant
💡 What: Replaced the sequential
for...ofloop awaiting promises with a fail-fast, concurrent structure utilizingPromise.race([p, criticalErrorPromise]).🎯 Why: The previous logic awaited promises in sequence. While
Promise.allSettledwas an alternative, it caused severe regressions because it waited for all promises, even when early exit logic (events.length < 100) or critical errors like RateLimit were met. ThecriticalErrorPromiseallows us to immediately break and short-circuit the execution while still processing concurrent resolves sequentially for safe list building.📊 Measured Improvement:
PR created automatically by Jules for task 729205816111143846 started by @is0692vs
Greptile Summary
fetchActivityで全ページリクエストを並行起動し、Promise.race([p, criticalErrorPromise])を使ってUserNotFoundError/RateLimitError発生時にループを即座に脱出できる fail-fast 構造に変更した PR です。その他の変更はコードフォーマット（行折り返し）のみです。criticalRejectをlet criticalReject: (reason?: unknown) => void;と宣言しているが、strict: trueの TypeScript はコールバック内の同期代入を認識できないため TS2454 コンパイルエラーが発生する可能性がある。promises.forEachの抑制ハンドラーが、ループで現在処理中のページのエラーも独立してキャッチするため、非クリティカルエラー時に二重ログが起きる可能性がある（機能的な影響なし、元のコードからの継続動作）。Confidence Score: 3/5
criticalRejectの宣言方法に起因するコンパイルエラーが解消されるまでマージは推奨しません。let criticalReject: (reason?: unknown) => voidという宣言は、strict: trueが有効な TypeScript では Promise コンストラクター内の同期代入を制御フロー解析で追えないため TS2454 エラーになる可能性が高く、ビルドが壊れるリスクがあります。ロジック自体（Promise.race による fail-fast）は意図どおりに動作しますが、このコンパイル上の文題が解消されないと安全にマージできません。src/lib/github.ts の
fetchActivity関数内のcriticalReject宣言箇所（749行目付近）を重点的に確認してください。Important Files Changed
fetchActivityのfor...ofループをPromise.race([p, criticalErrorPromise])による fail-fast 並行処理に置き換え。残りの変更はコードフォーマットのみ。criticalRejectの確定代入アサーション漏れによりstrict: true下でコンパイルエラーが発生する可能性あり。Sequence Diagram
sequenceDiagram participant ForLoop as fetchActivity participant Race as Promise.race participant P1 as Page1 Request participant P2 as Page2 Request participant CEP as criticalErrorPromise par Concurrent Launch P1-->>P1: fetch page 1 P2-->>P2: fetch page 2 end Note over P2: RateLimitError occurs P2->>CEP: criticalReject(error) P2-->>P2: throw error ForLoop->>Race: await Promise.race([p1, CEP]) CEP-->>Race: reject with RateLimitError Race-->>ForLoop: throw RateLimitError ForLoop-->>ForLoop: "catch rethrows -> early exit"Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "perf: optimize concurrent fetching early..." | Re-trigger Greptile