⚡ Bolt: Remove redundant network fetch for project lists by filtering global lists in-memory#145
Conversation
Co-authored-by: aicoder2009 <127642633+aicoder2009@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.
|
📝 WalkthroughWalkthroughThis PR optimizes ProjectDetailPage's data loading by reducing concurrent API requests from three to two. Instead of fetching a project-specific lists subset via a dedicated endpoint, the page now fetches the global lists collection and filters it in-memory to extract project-scoped results. The optimization pattern is documented in the decision log. ChangesIn-Memory Filtering to Reduce API Calls
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes a redundant GET request on the Project detail page by deriving “project lists” from the already-fetched global lists payload, and documents the pattern in the project’s Jules/Bolt notes.
Changes:
- Removed the
/api/projects/[id]/listsGET fetch fromProjectDetailPageand instead filters the/api/listsresponse in-memory to populatelists. - Updated
.jules/bolt.mdwith guidance about avoiding redundant subset fetches when the global collection is already available.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/app/projects/[id]/page.tsx |
Drops the redundant project-lists fetch and derives the project’s list subset by filtering the global lists response. |
.jules/bolt.md |
Adds a documented learning/action entry about filtering subsets in-memory to avoid redundant API calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (allListsResult.success) { | ||
| setAllLists(allListsResult.data); | ||
| // Filter the lists in-memory for this project to save an API request | ||
| setLists(allListsResult.data.filter((list: List) => list.projectId === projectId)); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/projects/[id]/page.tsx (2)
174-197:⚠️ Potential issue | 🟠 Major | ⚡ Quick winState synchronization bug:
allListsnot updated when removing list from project.When a list is removed from the project, only the
listsstate is updated (line 188), butallListsretains the staleprojectIdvalue. This causesavailableLists(computed at lines 232-235) to not include the removed list until the page is refreshed, preventing users from immediately re-adding it.🔄 Proposed fix to sync `allLists` state
if (result.success) { setLists((prev) => prev.filter((l) => l.id !== listId)); + setAllLists((prev) => + prev.map((l) => (l.id === listId ? { ...l, projectId: undefined } : l)) + ); posthog.capture("list_removed_from_project", { project_id: projectId });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/projects/`[id]/page.tsx around lines 174 - 197, When handleRemoveFromProject succeeds, also update allLists so the removed list no longer has the stale projectId; in the success branch (where setLists is called) call setAllLists and transform the matching item (by id) to have projectId: null (e.g., map over prev and replace the item with a shallow copy with projectId null), keeping setLists(prev => prev.filter(...)) and posthog.capture unchanged so availableLists recomputes correctly.
199-222:⚠️ Potential issue | 🟠 Major | ⚡ Quick winState synchronization bug:
allListsnot updated when adding list to project.When an existing list is added to the project, the
allListsstate is not updated to reflect the newprojectId. This causes the list to potentially remain inavailableLists(computed at lines 232-235) with stale data, leading to confusing UI behavior until the page is refreshed.🔄 Proposed fix to sync `allLists` state
if (result.success) { const addedList = allLists.find((l) => l.id === listId); if (addedList) { setLists((prev) => [...prev, { ...addedList, projectId }]); + setAllLists((prev) => + prev.map((l) => (l.id === listId ? { ...l, projectId } : l)) + ); } posthog.capture("list_added_to_project", { project_id: projectId });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/projects/`[id]/page.tsx around lines 199 - 222, When an existing list is added in handleAddExistingList, update the global lists state (allLists) as well as the project-specific lists so availableLists reflects the new projectId; after a successful response (where you currently call setLists), also call setAllLists and replace the matching entry (find by id) with a copy that includes projectId (or map over allLists to update the matching list), ensuring both setLists and setAllLists are kept in sync so availableLists (computed from allLists) no longer shows stale data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/projects/`[id]/page.tsx:
- Around line 174-197: When handleRemoveFromProject succeeds, also update
allLists so the removed list no longer has the stale projectId; in the success
branch (where setLists is called) call setAllLists and transform the matching
item (by id) to have projectId: null (e.g., map over prev and replace the item
with a shallow copy with projectId null), keeping setLists(prev =>
prev.filter(...)) and posthog.capture unchanged so availableLists recomputes
correctly.
- Around line 199-222: When an existing list is added in handleAddExistingList,
update the global lists state (allLists) as well as the project-specific lists
so availableLists reflects the new projectId; after a successful response (where
you currently call setLists), also call setAllLists and replace the matching
entry (find by id) with a copy that includes projectId (or map over allLists to
update the matching list), ensuring both setLists and setAllLists are kept in
sync so availableLists (computed from allLists) no longer shows stale data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 70b6d269-f959-4087-811a-eca9e6bebe6d
📒 Files selected for processing (2)
.jules/bolt.mdsrc/app/projects/[id]/page.tsx
💡 What: Modified
src/app/projects/[id]/page.tsxto stop making an extra, redundant API request to/api/projects/[id]/lists. Instead, the application now derives this subset in-memory by filtering the global response from/api/lists(which is already being fetched concurrently).🎯 Why: To reduce redundant network requests, decrease backend load, and slightly improve TTFB by not making the database run an isolated query for a subset of data that the client already receives globally in the same component.
📊 Impact:
🔬 Measurement:
/api/listsis called, but/api/projects/[id]/listsis no longer called, while the project lists still render correctly in the UI.Added a reflection on this codebase-specific anti-pattern to
.jules/bolt.md.PR created automatically by Jules for task 3159117257274641856 started by @aicoder2009
Summary by CodeRabbit
Documentation
Refactor