⚡ Bolt: [Performance] Optimize sitemap generation with parallel execution#155
⚡ Bolt: [Performance] Optimize sitemap generation with parallel execution#155
Conversation
Refactored sitemap generation to use `Promise.all` instead of sequential `for...of` loops, fetching data concurrently to speed up generation time while maintaining deterministic output order. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@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. |
📝 WalkthroughWalkthroughThe PR documents a behavior change for build-critical sitemap generation and refactors Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoOptimize sitemap generation with parallel data fetching
WalkthroughsDescription• Refactored sitemap generation to use Promise.all for parallel data fetching • Fetches getSpeakers and getTalks concurrently per year instead sequentially • Restructured code to map over years and collect results, improving performance • Reduces time complexity from O(N * M) sequential to O(M) concurrent operations Diagramflowchart LR
A["Sequential for...of loop"] -->|"Refactor"| B["Promise.all mapping"]
B -->|"Parallel fetching"| C["getSpeakers & getTalks concurrently"]
C -->|"Flatten results"| D["Combined sitemap output"]
File Changes1. app/sitemap.ts
|
Code Review by Qodo
|
![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 refactors the sitemap generation to use parallel data fetching for improved performance and adds a new policy to the documentation regarding the handling of API errors in build-critical files. A review comment notes that the current implementation of data fetching functions may still be swallowing exceptions, which contradicts the newly established policy to let errors propagate and fail the build during outages.
| for (const talk of group.sessions) { | ||
| urls.push({ | ||
| url: `${baseUrl}/${year}/talks/${talk.id}`, | ||
| const [speakers, sessionGroups] = await Promise.all([getSpeakers(year), getTalks(year)]); |
There was a problem hiding this comment.
The implementation of parallel fetching here relies on getSpeakers and getTalks, which currently swallow exceptions and return empty arrays on failure (as seen in the provided context for hooks/useSpeakers.ts and hooks/useTalks.ts). This directly conflicts with the new policy being established in .jules/bolt.md (lines 23-24) regarding 'Silently Failing Sitemaps'. To ensure the build fails accurately upon API outages as intended, these hooks should be refactored to propagate errors, or this block should validate the responses to ensure they are not empty due to a fetch failure.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/sitemap.ts (1)
33-34: Consider bounding year-level concurrency to avoid request bursts.
Promise.all(years.map(...))fires all edition fetches at once; as editions grow, this can create avoidable upstream throttling risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sitemap.ts` around lines 33 - 34, The current Promise.all(years.map(...)) in sitemap generation launches all year-level fetches concurrently (nestedUrls), which risks request bursts; replace it with a bounded-concurrency mapper (or batching) so only a fixed number (e.g., 5-10) of year fetches run in parallel. Locate the years.map callback used to build nestedUrls and swap the Promise.all(years.map(...)) call for a concurrency-controlled implementation (for example, using p-map with a concurrency option, or an internal async pool/batched for-loop) so the function that fetches editions per year runs with the chosen concurrency limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/sitemap.ts`:
- Around line 54-55: The sitemap currently proceeds when getSpeakers(year) or
getTalks(year) fail (they return []), so wrap the
Promise.all([getSpeakers(year), getTalks(year)]) call in a try/catch and
validate the results (speakers and sessionGroups) before generating URLs; if
either is missing/empty, throw a descriptive error to fail the build (or rethrow
the caught error) so app/sitemap.ts doesn't emit a partial sitemap. Reference
the getSpeakers and getTalks calls and the speakers/sessionGroups variables when
adding the validation and error handling.
---
Nitpick comments:
In `@app/sitemap.ts`:
- Around line 33-34: The current Promise.all(years.map(...)) in sitemap
generation launches all year-level fetches concurrently (nestedUrls), which
risks request bursts; replace it with a bounded-concurrency mapper (or batching)
so only a fixed number (e.g., 5-10) of year fetches run in parallel. Locate the
years.map callback used to build nestedUrls and swap the
Promise.all(years.map(...)) call for a concurrency-controlled implementation
(for example, using p-map with a concurrency option, or an internal async
pool/batched for-loop) so the function that fetches editions per year runs with
the chosen concurrency limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8b0dd79-6726-49e5-83c7-43f809ec9cfb
📒 Files selected for processing (2)
.jules/bolt.mdapp/sitemap.ts
| const [speakers, sessionGroups] = await Promise.all([getSpeakers(year), getTalks(year)]); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether sitemap data helpers swallow failures by returning [].
fd -i 'useSpeakers.ts' -t f --exec sed -n '1,120p' {}
fd -i 'useTalks.ts' -t f --exec sed -n '1,140p' {}
rg -n "catch\s*\(|return\s*\[\]|Failed to fetch" --type tsRepository: anyulled/devbcn-nextjs
Length of output: 8362
🏁 Script executed:
fd -i 'sitemap.ts' -t f --exec cat -n {}Repository: anyulled/devbcn-nextjs
Length of output: 3285
Sitemap generation silently succeeds with partial data during fetch failures.
At line 54, getSpeakers(year) and getTalks(year) are called without error handling. Both helpers catch failures and return empty arrays (see hooks/useSpeakers.ts and hooks/useTalks.ts), allowing the sitemap to be generated incomplete if either fetch fails. The sitemap then iterates these arrays (lines 56–73), generating fewer URLs than expected without failing the build. During outages, this results in a degraded sitemap being deployed instead of build failure.
Either wrap the Promise.all in try/catch to fail the build on critical fetch errors, add explicit validation that speakers and sessionGroups are non-empty before generating URLs, or ensure the helpers throw on errors instead of returning empty arrays.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/sitemap.ts` around lines 54 - 55, The sitemap currently proceeds when
getSpeakers(year) or getTalks(year) fail (they return []), so wrap the
Promise.all([getSpeakers(year), getTalks(year)]) call in a try/catch and
validate the results (speakers and sessionGroups) before generating URLs; if
either is missing/empty, throw a descriptive error to fail the build (or rethrow
the caught error) so app/sitemap.ts doesn't emit a partial sitemap. Reference
the getSpeakers and getTalks calls and the speakers/sessionGroups variables when
adding the validation and error handling.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| for (const talk of group.sessions) { | ||
| urls.push({ | ||
| url: `${baseUrl}/${year}/talks/${talk.id}`, | ||
| const [speakers, sessionGroups] = await Promise.all([getSpeakers(year), getTalks(year)]); |
There was a problem hiding this comment.
1. Sitemap hides fetch failures 🐞 Bug ☼ Reliability
sitemap() uses getSpeakers()/getTalks() inside Promise.all, but both helpers swallow network/non-OK errors and return empty arrays, so /sitemap.xml can be generated missing dynamic URLs without failing the build/request. This can lead to silently incomplete sitemaps (SEO impact) during upstream outages.
Agent Prompt
## Issue description
`sitemap()` can silently generate an incomplete sitemap because `getSpeakers()` and `getTalks()` swallow failures (returning `[]` on errors/non-OK responses). This prevents CI/builds (and runtime sitemap generation) from failing loudly when upstream Sessionize is unavailable.
## Issue Context
- `app/sitemap.ts` uses `getSpeakers(year)` and `getTalks(year)` to build dynamic URLs.
- `hooks/useSpeakers.ts` and `hooks/useTalks.ts` both `try/catch` and also return `[]` when `response.ok` is false.
- `.jules/bolt.md` explicitly calls out this failure mode as harmful for sitemaps.
## Fix Focus Areas
- Create strict variants that throw on failure (e.g., `getSpeakersOrThrow`, `getTalksOrThrow`) and use them from sitemap generation.
- Alternatively, add an option parameter to `getSpeakers/getTalks` (e.g., `{ swallowErrors: boolean }`) and pass `false` from `sitemap()`.
- Ensure non-OK responses throw (include status/statusText) so `Promise.all` rejects and the build/request fails.
### Fix Focus Areas (code references)
- app/sitemap.ts[33-75]
- hooks/useSpeakers.ts[14-28]
- hooks/useTalks.ts[16-30]
- .jules/bolt.md[21-24]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
💡 What:
Refactored the data fetching within
app/sitemap.tsfrom a sequentialfor...ofloop to usePromise.allmapping over the list of editions, fetchinggetSpeakersandgetTalkssimultaneously for each year.🎯 Why:
Previously, the sitemap generation created a waterfall sequence of API calls (fetching speakers then talks for one year, waiting, and proceeding to the next year sequentially). This introduces unnecessary I/O delays and degrades server response times when rendering sitemaps for SEO bots or during SSG build generation.
📊 Impact:
Significantly reduces execution time during both dynamic generation and static build time by performing independent API calls concurrently. Expected time complexity to fetch resources changes from O(N * M) sequentially to roughly O(M) concurrently where N is the number of years.
🔬 Measurement:
Can be verified by benchmarking the TTFB on the
/sitemap.xmlroute or tracking Next.js build times locally usingnpm run buildwith timing logs around the sitemap resolution block.PR created automatically by Jules for task 17187526585764795411 started by @anyulled
Summary by CodeRabbit
Performance Improvements
Reliability & Error Handling