⚡ Bolt: Optimize sitemap generation using Promise.all#80
⚡ Bolt: Optimize sitemap generation using Promise.all#80
Conversation
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 focuses on enhancing the performance and reliability of the application's sitemap generation. By transitioning from sequential data fetching to a concurrent approach, the system can now process information more efficiently, leading to faster build times. Additionally, the inclusion of error handling mechanisms ensures that the sitemap generation remains robust even when encountering issues with external data sources. Highlights
Changelog
Ignored Files
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request contains two independent changes: a Cypress test selector update and the removal of a minimatch dependency override. The first modifies which DOM elements are targeted in the homepage editions test, while the second eliminates an explicit version constraint from the dependency overrides configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
![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 effectively optimizes the sitemap generation by parallelizing data fetching for each year using Promise.all. This is a great improvement for performance. My review includes a suggestion to further refine the implementation by adopting a more functional pattern that avoids mutating shared state from within concurrent operations, which enhances code robustness and maintainability.
| await Promise.all( | ||
| years.map(async (year) => { | ||
| const yearUrls: MetadataRoute.Sitemap = []; | ||
|
|
||
| const yearPages = ["speakers", "talks", "schedule", "job-offers", "cfp", "diversity", "sponsorship", "travel"]; | ||
| for (const page of yearPages) { | ||
| urls.push({ | ||
| url: `${baseUrl}/${year}/${page}`, | ||
| yearUrls.push({ | ||
| url: `${baseUrl}/${year}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "weekly", | ||
| priority: 0.8, | ||
| changeFrequency: "daily", | ||
| priority: 0.9, | ||
| }); | ||
| } | ||
|
|
||
| const speakers = await getSpeakers(year); | ||
| for (const speaker of speakers) { | ||
| urls.push({ | ||
| url: `${baseUrl}/${year}/speakers/${speaker.id}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "weekly", | ||
| priority: 0.7, | ||
| }); | ||
| } | ||
| const yearPages = ["speakers", "talks", "schedule", "job-offers", "cfp", "diversity", "sponsorship", "travel"]; | ||
| for (const page of yearPages) { | ||
| yearUrls.push({ | ||
| url: `${baseUrl}/${year}/${page}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "weekly", | ||
| priority: 0.8, | ||
| }); | ||
| } | ||
|
|
||
| // Fetch speakers and talks in parallel for each year, gracefully handling failures | ||
| const [speakers, sessionGroups] = await Promise.all([getSpeakers(year).catch(() => []), getTalks(year).catch(() => [])]); | ||
|
|
||
| const sessionGroups = await getTalks(year); | ||
| for (const group of sessionGroups) { | ||
| for (const talk of group.sessions) { | ||
| urls.push({ | ||
| url: `${baseUrl}/${year}/talks/${talk.id}`, | ||
| for (const speaker of speakers) { | ||
| yearUrls.push({ | ||
| url: `${baseUrl}/${year}/speakers/${speaker.id}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "weekly", | ||
| priority: 0.7, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| const companies = getJobOffersByYear(year); | ||
| for (const company of companies) { | ||
| urls.push({ | ||
| url: `${baseUrl}/${year}/job-offers/${slugify(company.name)}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "monthly", | ||
| priority: 0.5, | ||
| }); | ||
| } | ||
| } | ||
| for (const group of sessionGroups) { | ||
| for (const talk of group.sessions) { | ||
| yearUrls.push({ | ||
| url: `${baseUrl}/${year}/talks/${talk.id}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "weekly", | ||
| priority: 0.7, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| const companies = getJobOffersByYear(year); | ||
| for (const company of companies) { | ||
| yearUrls.push({ | ||
| url: `${baseUrl}/${year}/job-offers/${slugify(company.name)}`, | ||
| lastModified: new Date(), | ||
| changeFrequency: "monthly", | ||
| priority: 0.5, | ||
| }); | ||
| } | ||
|
|
||
| urls.push(...yearUrls); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
While the current implementation with urls.push(...yearUrls) inside the map callback will likely work correctly in Node.js's single-threaded event loop model, it relies on mutating a shared array from within concurrent asynchronous operations. This is generally considered an unsafe pattern that can be fragile and harder to reason about.
A more robust and functional approach is to have each asynchronous operation return its result, and then aggregate the results once all promises have been resolved. This avoids shared mutable state and makes the data flow more explicit.
const yearlyUrls = await Promise.all(
years.map(async (year) => {
const yearUrls: MetadataRoute.Sitemap = [];
yearUrls.push({
url: `${baseUrl}/${year}`,
lastModified: new Date(),
changeFrequency: "daily",
priority: 0.9,
});
const yearPages = ["speakers", "talks", "schedule", "job-offers", "cfp", "diversity", "sponsorship", "travel"];
for (const page of yearPages) {
yearUrls.push({
url: `${baseUrl}/${year}/${page}`,
lastModified: new Date(),
changeFrequency: "weekly",
priority: 0.8,
});
}
// Fetch speakers and talks in parallel for each year, gracefully handling failures
const [speakers, sessionGroups] = await Promise.all([getSpeakers(year).catch(() => []), getTalks(year).catch(() => [])]);
for (const speaker of speakers) {
yearUrls.push({
url: `${baseUrl}/${year}/speakers/${speaker.id}`,
lastModified: new Date(),
changeFrequency: "weekly",
priority: 0.7,
});
}
for (const group of sessionGroups) {
for (const talk of group.sessions) {
yearUrls.push({
url: `${baseUrl}/${year}/talks/${talk.id}`,
lastModified: new Date(),
changeFrequency: "weekly",
priority: 0.7,
});
}
}
const companies = getJobOffersByYear(year);
for (const company of companies) {
yearUrls.push({
url: `${baseUrl}/${year}/job-offers/${slugify(company.name)}`,
lastModified: new Date(),
changeFrequency: "monthly",
priority: 0.5,
});
}
return yearUrls;
})
);
urls.push(...yearlyUrls.flat());
Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
![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)
32-87: Keep sitemap ordering deterministic while parallelizing by year.
urls.push(...yearUrls)from inside the parallel callbacks makes the final array order depend on whichever year's fetch finishes first. The data is still correct, but the sitemap output becomes unstable across runs and can create noisy diffs/cache churn. Return eachyearUrlsarray fromPromise.alland flatten afterward to preserveyearsorder.♻️ Suggested refactor
- await Promise.all( + const urlsByYear = await Promise.all( years.map(async (year) => { const yearUrls: MetadataRoute.Sitemap = []; ... - urls.push(...yearUrls); + return yearUrls; }) ); + + urls.push(...urlsByYear.flat());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sitemap.ts` around lines 32 - 87, The current parallel map pushes yearUrls into the shared urls array from inside each async callback, making final sitemap order nondeterministic; instead have the async callback (the years.map async function that calls getSpeakers and getTalks) return its yearUrls array, await Promise.all to get an array of yearUrls results in the original years order, then flatten/concat those results into urls (e.g., const perYear = await Promise.all(...); urls.push(...perYear.flat());) so ordering is stable while still parallelizing fetches like getSpeakers and getTalks.
🤖 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 12-16: The workflow pins third-party actions using mutable tags
(actions/checkout@v6 and markmishaev76/ai-harness-scorecard@v1); replace each
tag with the corresponding full commit SHA by looking up the exact commit in the
action repos and updating the uses entries to the full 40-character commit SHA
for markmishaev76/ai-harness-scorecard and for actions/checkout (or at minimum
for the third-party action) so the workflow references an immutable commit;
ensure you commit the updated workflow to the repo.
---
Nitpick comments:
In `@app/sitemap.ts`:
- Around line 32-87: The current parallel map pushes yearUrls into the shared
urls array from inside each async callback, making final sitemap order
nondeterministic; instead have the async callback (the years.map async function
that calls getSpeakers and getTalks) return its yearUrls array, await
Promise.all to get an array of yearUrls results in the original years order,
then flatten/concat those results into urls (e.g., const perYear = await
Promise.all(...); urls.push(...perYear.flat());) so ordering is stable while
still parallelizing fetches like getSpeakers and getTalks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffa62f18-1e8d-4e20-a7a9-e64af587708d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/scorecard.ymlapp/sitemap.tscypress/e2e/home/home-editions.cy.tsscorecard-badge.jsonscorecard-report.md
.github/workflows/scorecard.yml
Outdated
| permissions: | ||
| contents: write | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - uses: markmishaev76/ai-harness-scorecard@v1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and inspect the scorecard.yml file
find . -name "scorecard.yml" -o -name "*scorecard*" | grep -E "\.(yml|yaml)$" | head -20Repository: anyulled/devbcn-nextjs
Length of output: 99
🏁 Script executed:
# Check if .github/workflows/scorecard.yml exists and read it
if [ -f .github/workflows/scorecard.yml ]; then
cat -n .github/workflows/scorecard.yml
else
echo "File not found at .github/workflows/scorecard.yml"
# Try to find it in the repo
find . -name "*scorecard*.yml" -o -name "*scorecard*.yaml"
fiRepository: anyulled/devbcn-nextjs
Length of output: 894
🌐 Web query:
GitHub Actions security hardening pin third-party actions commit SHA
💡 Result:
To harden GitHub Actions against supply‑chain attacks, pin any third‑party action (and reusable workflow) to an immutable reference—preferably a full commit SHA rather than a mutable tag like @v1 or a branch like @main. GitHub explicitly notes that pinning to a full‑length commit SHA is currently the only way to use an action as an immutable release. [1]
Why pin to a commit SHA (vs tags)
- Tags and branches can move (retagging/force-push), so
uses: owner/action@v1may execute different code later without changing your workflow file. GitHub recommends SHA pinning to reduce impact of compromised dependencies. [1][2] - SHA pinning helps prevent surprise changes and reduces risk from malicious tag updates. [3]
How to pin (example)
Instead of:
- uses: actions/checkout@v4Use the exact commit:
- uses: actions/checkout@<FULL_LENGTH_COMMIT_SHA>(Replace with the commit that corresponds to the release you reviewed.) [1][3]
Enforce SHA pinning
- GitHub provides repository/org/enterprise policies to require actions be pinned to a full-length commit SHA. [1]
- You can also add a workflow check that fails PRs if actions aren’t SHA‑pinned (example: “Ensure SHA Pinned Actions”). [4]
Keeping pinned SHAs updated safely
- Treat action updates like dependencies: periodically bump the pinned SHA to the next vetted release (often via Dependabot/automation + code review). This preserves immutability while still getting fixes. GitHub’s governance features and guidance emphasize proactive pinning and controlled updates. [2]
Sources
- GitHub Docs — Security hardening for GitHub Actions (pinning to full-length commit SHA; policies) https://docs.github.com/en/enterprise-cloud@latest/actions/how-tos/security-for-github-actions/security-guides/security-hardening-for-github-actions
- GitHub Changelog (2025‑08‑15) — Actions policy now supports blocking and SHA pinning actions https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- GitHub Well‑Architected — Actions security recommendations (pin to immutable reference; commit SHA example) https://wellarchitected.github.com/library/application-security/recommendations/actions-security/
- GitHub Marketplace — Ensure SHA Pinned Actions https://github.com/marketplace/actions/ensure-sha-pinned-actions
Pin the third-party action to an immutable commit SHA.
This workflow grants contents: write permission to a third-party action and then commits and pushes back to the repository. GitHub's security guidance recommends pinning all third-party actions to a full-length commit SHA because tags like @v1 are mutable and can be retagged; a compromised action could exploit write access to introduce malicious code into your repository. Commit SHAs are the only immutable reference for actions.
🔒 Suggested hardening
- - uses: markmishaev76/ai-harness-scorecard@v1
+ - uses: markmishaev76/ai-harness-scorecard@<full-length-commit-sha> # v1
id: scorecard🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/scorecard.yml around lines 12 - 16, The workflow pins
third-party actions using mutable tags (actions/checkout@v6 and
markmishaev76/ai-harness-scorecard@v1); replace each tag with the corresponding
full commit SHA by looking up the exact commit in the action repos and updating
the uses entries to the full 40-character commit SHA for
markmishaev76/ai-harness-scorecard and for actions/checkout (or at minimum for
the third-party action) so the workflow references an immutable commit; ensure
you commit the updated workflow to the repo.
…lock.json Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
1 participant
💡 What: Refactored
app/sitemap.tsto fetch speakers and talks data concurrently usingPromise.allacross years and resource types. Also added fallback empty arrays to handle partial API failures without crashing the entire sitemap generation.🎯 Why: Sequential
awaitstatements inside the loops caused significant I/O blocking, making sitemap generation time linearly dependent on the number of years and resources.📊 Impact: drastically reduces sitemap generation time by overlapping network and I/O wait times.
🔬 Measurement: Compare sitemap generation build times before and after this change.
PR created automatically by Jules for task 14425354385331826874 started by @anyulled
Summary by CodeRabbit
Tests
Chores