fix(accounts): compact legacy stored account comments#44
Merged
tomcasaburi merged 2 commits intomasterfrom Mar 20, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds per-account comment storage versioning and a migration that sanitizes legacy Changes
Sequence DiagramsequenceDiagram
participant Client
participant AccountsDB as accounts-database
participant IndexedDB
Client->>AccountsDB: getAccountComments(accountId) / getExportedAccountJson / deleteAccountComment / addAccountComment
activate AccountsDB
AccountsDB->>AccountsDB: ensureAccountCommentsDatabaseLayout(accountId)
AccountsDB->>IndexedDB: read __storageVersion for account
IndexedDB-->>AccountsDB: version (or undefined)
alt missing or legacy version
AccountsDB->>IndexedDB: read all stored comments
IndexedDB-->>AccountsDB: comments (may contain replies.pages)
AccountsDB->>AccountsDB: sanitize comments (remove replies.pages, keep pageCids)
AccountsDB->>IndexedDB: write sanitized comments + __storageVersion = 1
end
AccountsDB->>IndexedDB: proceed with requested operation (read/add/delete/export)
IndexedDB-->>AccountsDB: result
AccountsDB-->>Client: return result (compacted/sanitized)
deactivate AccountsDB
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/stores/accounts/accounts-database.test.ts`:
- Around line 762-791: Add a regression test that verifies mutation compaction
for legacy accountComments: create a legacy-style entry in the per-account
"accountComments" DB via createPerAccountDatabase (include a replies.pages
structure and pageCids), then call the mutating API under test (e.g.,
addAccountComment or deleteAccountComment for the account id) and assert that
after the mutation the stored row has been compacted (replies.pages removed,
pageCids preserved) and that __storageVersion is updated; also verify exported
JSON from accountsDatabase.getExportedAccountJson reflects the compacted shape.
In `@src/stores/accounts/accounts-database.ts`:
- Around line 366-386: ensureAccountCommentsDatabaseLayout can run concurrently
and clobber concurrent edits and can mark the DB upgraded on partial failure;
serialize the migration by acquiring a per-account migration lock (e.g., a
migration key in accountCommentsDatabase) before snapshotting via
getDatabaseAsArray, abort if lock exists or wait/retry, then compute
updatedComments and for each index perform a compare-and-set write that only
replaces the row if the current stored row still equals the original snapshot
(use the snapshot from getDatabaseAsArray), perform all row rewrites and only
after all succeed set storageVersionKey to commentStorageVersion, and finally
release the migration lock in a finally block; reference
ensureAccountCommentsDatabaseLayout, accountCommentsDatabase, storageVersionKey,
commentStorageVersion, getDatabaseAsArray, sanitizeStoredAccountComment, and
isEqual when making these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc0c0d30-755d-4aca-ae2d-447d1871e00d
📒 Files selected for processing (2)
src/stores/accounts/accounts-database.test.tssrc/stores/accounts/accounts-database.ts
Member
Author
|
Addressed the valid review findings in
Local verification after the fix:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/stores/accounts/accounts-database.ts (2)
458-464: Optional: remove redundant sanitization after migration.After
ensureAccountCommentsDatabaseLayoutcompletes, all persisted comments are already sanitized. ThesanitizeStoredAccountCommentcall at line 460 is idempotent but adds CPU overhead on every read.Since comments are only mutated via
addAccountComment(which already sanitizes before writing), this sanitization could be removed. TheindexandaccountIdassignments would remain.♻️ Suggested simplification
// add index and account id to account comments for easier updating for (const i in comments) { - comments[i] = sanitizeStoredAccountComment(comments[i]); comments[i].index = Number(i); comments[i].accountId = accountId; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-database.ts` around lines 458 - 464, Remove the redundant call to sanitizeStoredAccountComment in the read path: since ensureAccountCommentsDatabaseLayout guarantees persisted comments are sanitized and addAccountComment sanitizes before write, delete the sanitizeStoredAccountComment(comments[i]) invocation in the loop that sets comments[i].index and comments[i].accountId; keep the index and accountId assignments intact and ensure no other callers rely on this extra sanitization in the function that returns comments.
432-441: Consider removing redundantstorageVersionKeywrites.After
ensureAccountCommentsDatabaseLayout(accountId)completes at line 424,storageVersionKeyis already set tocommentStorageVersion. Writing it again in every add/edit operation adds unnecessary I/O overhead.The migration guarantees the version is set, so these redundant writes could be removed unless you specifically want to guard against external DB modifications.
♻️ Suggested simplification
if (typeof accountCommentIndex === "number") { assert( accountCommentIndex < length, `addAccountComment cannot edit comment no comment in database at accountCommentIndex '${accountCommentIndex}'`, ); - await Promise.all([ - accountCommentsDatabase.setItem(String(accountCommentIndex), comment), - accountCommentsDatabase.setItem(storageVersionKey, commentStorageVersion), - ]); + await accountCommentsDatabase.setItem(String(accountCommentIndex), comment); } else { await Promise.all([ accountCommentsDatabase.setItem(String(length), comment), - accountCommentsDatabase.setItem(storageVersionKey, commentStorageVersion), accountCommentsDatabase.setItem("length", length + 1), ]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-database.ts` around lines 432 - 441, The code is redundantly writing storageVersionKey = commentStorageVersion on every add/edit; since ensureAccountCommentsDatabaseLayout(accountId) already sets storageVersionKey, remove the accountCommentsDatabase.setItem(storageVersionKey, commentStorageVersion) calls from both Promise.all branches around accountCommentsDatabase.setItem(String(...), comment) and accountCommentsDatabase.setItem("length", ...), leaving only the item/length writes; if you want to guard against external DB tampering instead, replace the duplicate writes with a conditional check that verifies the current storageVersionKey before proceeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/stores/accounts/accounts-database.ts`:
- Around line 458-464: Remove the redundant call to sanitizeStoredAccountComment
in the read path: since ensureAccountCommentsDatabaseLayout guarantees persisted
comments are sanitized and addAccountComment sanitizes before write, delete the
sanitizeStoredAccountComment(comments[i]) invocation in the loop that sets
comments[i].index and comments[i].accountId; keep the index and accountId
assignments intact and ensure no other callers rely on this extra sanitization
in the function that returns comments.
- Around line 432-441: The code is redundantly writing storageVersionKey =
commentStorageVersion on every add/edit; since
ensureAccountCommentsDatabaseLayout(accountId) already sets storageVersionKey,
remove the accountCommentsDatabase.setItem(storageVersionKey,
commentStorageVersion) calls from both Promise.all branches around
accountCommentsDatabase.setItem(String(...), comment) and
accountCommentsDatabase.setItem("length", ...), leaving only the item/length
writes; if you want to guard against external DB tampering instead, replace the
duplicate writes with a conditional check that verifies the current
storageVersionKey before proceeding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5cc3c0e2-b4d9-4c60-8ed7-179230687b3c
📒 Files selected for processing (2)
src/stores/accounts/accounts-database.test.tssrc/stores/accounts/accounts-database.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/stores/accounts/accounts-database.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Compact legacy
accountCommentsrecords the first time the new hooks read, mutate, or export them so existing users stop carrying old bloated reply payloads in storage.Closes #43
Note
Medium Risk
Touches IndexedDB persistence for
accountCommentsby adding an on-access migration/compaction step; bugs here could rewrite or drop stored comments, though it’s guarded by versioning and covered by tests.Overview
Adds automatic compaction/migration for legacy
accountCommentsstorage. A newensureAccountCommentsDatabaseLayoutrewrites stored comments to a compact form (droppingreplies.pageswhile keepingreplies.pageCids) and stamps a__storageVersionso the rewrite runs once per account.This migration is now triggered before
getAccountComments,addAccountComment,deleteAccountComment, andgetExportedAccountJson, with a per-account in-flight migration guard to avoid concurrent rewrites. Tests were added to verify legacy comments are compacted during export, read, and delete flows and that the DB is marked migrated.Written by Cursor Bugbot for commit 901887f. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Tests
Bug Fixes