⚡ Bolt: Parallelize sitemap data fetching for improved build performance#69
⚡ Bolt: Parallelize sitemap data fetching for improved build performance#69
Conversation
Replaced sequential `await` fetches for `getSpeakers` and `getTalks` inside the loop with `Promise.all` combined with `years.map` to fetch data concurrently for all years, significantly reducing the `sitemap.ts` execution and site build times. Added graceful fallbacks using `.catch` for independent promise resolution. Co-authored-by: anyulled <100741+anyulled@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.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a performance bottleneck in the sitemap generation process by transitioning from sequential data fetching to a parallel approach. By leveraging concurrent operations for retrieving speaker and talk data across multiple years, the overall build time for the application is substantially improved, particularly for static page generation. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughTwo dependency override entries are removed from the package.json overrides section: minimatch ^10.0.1 and ajv ^8.18.0. All other configuration remains unchanged. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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.
Code Review
This pull request effectively addresses the performance bottleneck in sitemap generation by parallelizing data fetching using Promise.all. The transition from a sequential for...of loop to a concurrent years.map is a significant improvement for build times. The code is well-structured, and the use of .catch to handle individual promise failures is a great touch for resilience. I have one suggestion to further improve data consistency in the generated sitemap.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/sitemap.ts (2)
51-52: Remove redundant per-call fallbacks aroundgetSpeakers/getTalks.Both helpers already catch/log and return
[], so these extra catches add noise and can hide future unexpected failures.♻️ Suggested simplification
- const [speakers, sessionGroups] = await Promise.all([getSpeakers(year).catch(() => []), getTalks(year).catch(() => [])]); + const [speakers, sessionGroups] = await Promise.all([getSpeakers(year), getTalks(year)]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sitemap.ts` around lines 51 - 52, The Promise.all call is wrapping getSpeakers and getTalks with redundant .catch(() => []) handlers; remove those extra per-call catches and call Promise.all([getSpeakers(year), getTalks(year)]) directly so the helpers' internal error handling (which already logs and returns []) is used; update the destructuring assignment that assigns to speakers and sessionGroups accordingly.
31-32: Good parallelization; consider adding a concurrency cap for year-level fan-out.This currently bursts up to
2 * years.lengthremote fetches at once. A small batch/concurrency limit would preserve performance while reducing backend/load-shedding risk as editions grow.Also applies to: 86-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sitemap.ts` around lines 31 - 32, The current years.map async fan-out (creating yearPromises) fires up to 2 * years.length concurrent remote fetches; change this to use a bounded concurrency executor (e.g., p-limit or a simple batch runner) so only N year tasks run at once (suggest 4–8), and apply the same bounded pattern to the other year-level fan-out site generation spot (the map at the later block around the other year/edition map). Locate the years.map usage that builds yearPromises and replace it with a limited-run wrapper (or processInBatches helper) that schedules the async job for each year under the concurrency cap, then await Promise.all on the controlled set; repeat the same change for the second years->editions fan-out referenced in the review.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/scorecard.yml:
- Around line 15-16: Replace the floating action tags with their immutable SHAs:
change the uses entry for actions/checkout@v6 to
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd and change
markmishaev76/ai-harness-scorecard@v1 to
markmishaev76/ai-harness-scorecard@dbb34ac123ef4a2c75b5b75d8789bc2420146c99 in
the workflow so the actions/checkout and markmishaev76/ai-harness-scorecard
references are pinned to the provided commit SHAs.
In @.jules/bolt.md:
- Line 18: Update the learning note in .jules/bolt.md to stop saying “blocked
the event loop” and instead state that the O(N) sequential loop in
app/sitemap.ts serialized network waits/requests (awaiting speaker and talk
fetching per year), which increased total runtime as the number of years grew;
edit the sentence to use “serialized network requests/waits” or similar phrasing
to accurately describe the performance impact.
---
Nitpick comments:
In `@app/sitemap.ts`:
- Around line 51-52: The Promise.all call is wrapping getSpeakers and getTalks
with redundant .catch(() => []) handlers; remove those extra per-call catches
and call Promise.all([getSpeakers(year), getTalks(year)]) directly so the
helpers' internal error handling (which already logs and returns []) is used;
update the destructuring assignment that assigns to speakers and sessionGroups
accordingly.
- Around line 31-32: The current years.map async fan-out (creating yearPromises)
fires up to 2 * years.length concurrent remote fetches; change this to use a
bounded concurrency executor (e.g., p-limit or a simple batch runner) so only N
year tasks run at once (suggest 4–8), and apply the same bounded pattern to the
other year-level fan-out site generation spot (the map at the later block around
the other year/edition map). Locate the years.map usage that builds yearPromises
and replace it with a limited-run wrapper (or processInBatches helper) that
schedules the async job for each year under the concurrency cap, then await
Promise.all on the controlled set; repeat the same change for the second
years->editions fan-out referenced in the review.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/scorecard.yml.jules/bolt.mdapp/sitemap.tsscorecard-badge.jsonscorecard-report.md
.github/workflows/scorecard.yml
Outdated
| - uses: actions/checkout@v6 | ||
| - uses: markmishaev76/ai-harness-scorecard@v1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repos_and_tags=(
"actions/checkout v6"
"markmishaev76/ai-harness-scorecard v1"
)
for item in "${repos_and_tags[@]}"; do
repo="$(awk '{print $1}' <<<"$item")"
tag="$(awk '{print $2}' <<<"$item")"
echo "== $repo @ $tag =="
url="https://api.github.com/repos/$repo/git/ref/tags/$tag"
if curl -fsSL "$url" >/tmp/ref.json; then
sha="$(jq -r '.object.sha' /tmp/ref.json)"
echo "resolved_sha=$sha"
else
echo "ERROR: tag '$tag' not found for $repo"
exit 1
fi
doneRepository: anyulled/devbcn-nextjs
Length of output: 247
🏁 Script executed:
cat -n .github/workflows/scorecard.ymlRepository: anyulled/devbcn-nextjs
Length of output: 894
Pin external actions to immutable SHAs.
Using floating tags (@v6, @v1) weakens supply-chain guarantees. Pin these actions to their resolved SHAs:
actions/checkout@v6→de0fac2e4500dabe0009e67214ff5f5447ce83ddmarkmishaev76/ai-harness-scorecard@v1→dbb34ac123ef4a2c75b5b75d8789bc2420146c99
Update lines 15-16 to use the SHAs instead of the floating tags.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/scorecard.yml around lines 15 - 16, Replace the floating
action tags with their immutable SHAs: change the uses entry for
actions/checkout@v6 to actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
and change markmishaev76/ai-harness-scorecard@v1 to
markmishaev76/ai-harness-scorecard@dbb34ac123ef4a2c75b5b75d8789bc2420146c99 in
the workflow so the actions/checkout and markmishaev76/ai-harness-scorecard
references are pinned to the provided commit SHAs.
Removed `ajv` and `minimatch` overrides which were causing test failures (`minimatch is not a function`) and lint errors (`TypeError: Cannot set properties of undefined (setting 'defaultMeta')`) in the GitHub Actions CI environment. Updated dependencies correctly to allow the tree to resolve natively. Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
💡 What: Refactored
app/sitemap.tsto fetch speakers and talks concurrently for each year usingPromise.alland.catch, and map over theyearsarray concurrently instead of sequentially awaiting in afor...ofloop.🎯 Why: The previous sequential data fetching for speakers and talks across multiple years (O(N) operations) caused slower sitemap generation and overall site build times. By doing this in parallel, we unblock the event loop and speed up Next.js's static generation process.
📊 Impact: Reduces time taken to build static pages (
generateStaticParams/sitemap.xml) proportionally to the number of editions/years available, making sitemap generation nearly O(1) in terms of API round-trips.🔬 Measurement: Verify by running
npm run buildand checking the sitemap generation speed compared to previous builds. Ensureapp/sitemap.tsURLs are accurately populated by checkingnpm run testor inspectingsitemap.xmloutput.PR created automatically by Jules for task 7265519895392991802 started by @anyulled
Summary by CodeRabbit