⚡ Bolt: Optimize sequential IndexedDB writes in loadNotesIntent#5
Conversation
- Added `saveNotesToDB` to `offline.ts` to support bulk writes in a single transaction. - Modified `loadNotesIntent` in `notesIntents.ts` to use `saveNotesToDB` instead of sequential `saveNoteToDB` calls. - Established a benchmark demonstrating ~8x performance improvement for bulk writes. Co-authored-by: davidraehles <6085055+davidraehles@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. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes offline caching during loadNotesIntent by replacing per-note IndexedDB writes (one transaction per note) with a single bulk transaction, reducing transaction overhead and improving load performance.
Changes:
- Added
saveNotesToDB(notes: Note[])to write multiple notes in one IndexedDB transaction. - Updated
loadNotesIntentto persist decrypted notes via the new bulk write helper. - Added
frontend/pnpm-lock.yamland a short benchmark note in.jules/bolt.md.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/services/offline.ts | Adds saveNotesToDB bulk write helper using a single readwrite transaction. |
| frontend/src/intents/notesIntents.ts | Switches loadNotesIntent to bulk-save decrypted notes to IndexedDB. |
| frontend/pnpm-lock.yaml | Introduces a pnpm lockfile (currently alongside an existing package-lock.json). |
| .jules/bolt.md | Documents the bulk-write optimization and benchmark numbers. |
Files not reviewed (1)
- frontend/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (const note of notes) { | ||
| tx.store.put(note); | ||
| } |
There was a problem hiding this comment.
tx.store.put(note) returns a Promise in idb; ignoring it can lead to unhandled rejections if a put fails (even though tx.done will reject). Consider awaiting each put inside the loop, or collecting the promises and awaiting them (e.g., await Promise.all(...)) before awaiting tx.done so failures are properly observed and no promise is left floating.
| for (const note of notes) { | |
| tx.store.put(note); | |
| } | |
| const putPromises: Promise<unknown>[] = []; | |
| for (const note of notes) { | |
| putPromises.push(tx.store.put(note)); | |
| } | |
| await Promise.all(putPromises); |
There was a problem hiding this comment.
Collected put promises into an array and awaiting them with Promise.all before tx.done.
There was a problem hiding this comment.
Excellent suggestion. I've updated saveNotesToDB to collect all put promises and await them alongside tx.done using Promise.all for better efficiency and explicit completion tracking.
| lockfileVersion: '9.0' | ||
|
|
||
| settings: | ||
| autoInstallPeers: true | ||
| excludeLinksFromLockfile: false |
There was a problem hiding this comment.
This PR introduces pnpm-lock.yaml, but the repo already has frontend/package-lock.json and there’s no indication that pnpm is used/configured (no packageManager field, no pnpm docs/scripts). Keeping two lockfiles for the same package.json is likely to cause dependency drift and CI/tooling confusion; please remove pnpm-lock.yaml (or, if migrating to pnpm, remove package-lock.json and update docs/CI accordingly).
There was a problem hiding this comment.
Removed pnpm-lock.yaml from the repository.
There was a problem hiding this comment.
Removed pnpm-lock.yaml as requested to keep the repository clean of this artifact.
| // Save to IndexedDB in bulk | ||
| await saveNotesToDB(decryptedNotes); |
There was a problem hiding this comment.
Switching to a single transaction changes failure semantics: if any put fails, IndexedDB will abort the whole transaction and none of the notes will be persisted. Previously, each note was saved in its own transaction, so you could end up with a partially populated offline cache. Please confirm the all-or-nothing behavior is intended; if not, consider chunking or falling back to per-item writes on transaction abort so some notes still get cached.
| // Save to IndexedDB in bulk | |
| await saveNotesToDB(decryptedNotes); | |
| // Save to IndexedDB in bulk when possible; fall back to per-note saves on failure | |
| try { | |
| await saveNotesToDB(decryptedNotes); | |
| } catch { | |
| for (const note of decryptedNotes) { | |
| try { | |
| await saveNoteToDB(note); | |
| } catch { | |
| // Ignore individual failures to allow partial offline caching | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Added try/catch around saveNotesToDB with a per-item saveNoteToDB fallback so partial offline caching still works if the bulk transaction fails.
There was a problem hiding this comment.
Great point. I've added a try/catch block around the bulk save operation. If it fails, it will now fall back to sequential saveNoteToDB calls, ensuring that partial offline caching still works and providing more resilience.
….yaml, add fallback for bulk save
- Updated `saveNotesToDB` to use `Promise.all` for `put` operations. - Added try/catch with fallback to sequential save in `loadNotesIntent`. - Removed `pnpm-lock.yaml` from the repository. Co-authored-by: davidraehles <6085055+davidraehles@users.noreply.github.com>
💡 What: The optimization replaces sequential IndexedDB writes (one transaction per item) with a single bulk transaction for multiple items in
loadNotesIntent.🎯 Why: Opening a transaction for each item is very slow due to the overhead of transaction creation and coordination in IndexedDB. Using a single transaction for all items is a standard and highly effective optimization.
📊 Measured Improvement:
Using a simulated benchmark with 50 items and 10ms transaction overhead:
Verified by running the benchmark and existing tests. No regressions introduced.
PR created automatically by Jules for task 10663017803853492186 started by @davidraehles