fix(accounts): speed up account backup imports - #80
Conversation
📝 WalkthroughWalkthroughAccount import now assigns fresh local identifiers, hydrates identities once, bulk-imports sanitized history, and initializes update watchers only for the ten newest imported comments. Database and action tests cover hydration, validation, round-tripping, and bulk-write behavior. ChangesAccount import
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ImportAccount
participant AccountsDatabase
participant HistoryStorage
ImportAccount->>AccountsDatabase: addAccount with returnHydratedAccount
AccountsDatabase-->>ImportAccount: hydrated account
ImportAccount->>AccountsDatabase: importAccountHistory
AccountsDatabase->>HistoryStorage: replace sanitized comments, votes, and edits
HistoryStorage-->>AccountsDatabase: indexed history
AccountsDatabase-->>ImportAccount: imported history
ImportAccount->>ImportAccount: initialize watchers for newest comments
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc73be676f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const existingAccountId = accountNamesToAccountIds?.[account.name]; | ||
| if (existingAccountId && existingAccountId !== account.id) { | ||
| throw Error(`account name '${account.name}' already exists in database`); |
There was a problem hiding this comment.
When an existing account is renamed through setAccount, addAccount adds the new accountNamesToAccountIds entry but does not remove the old name entry. The previous duplicate-name check loaded the actual accounts and compared their current names, so a later createAccount/import using the vacated old name worked; this metadata-only check now treats that stale old-name mapping as a real duplicate and rejects the operation. Please either clean/rebuild the name map on rename or verify that the mapped account still has this name before throwing.
Useful? React with 👍 / 👎.
| const accountToImport = { | ||
| ...imported.account, | ||
| communities, | ||
| id: generatedAccount.id, | ||
| id: uuid(), | ||
| }; |
There was a problem hiding this comment.
When importing an older backup that lacks fields now supplied by generateDefaultAccount (for example blockedAddresses, blockedCids, or subscriptions), this direct import no longer overlays those defaults before persisting the account. The imported account can then remain in store with undefined block maps, and paths such as useBlock/feed filtering index account.blockedAddresses[...] or account.blockedCids[...] directly, causing a runtime crash after an otherwise valid legacy backup import. Seed the imported account with current defaults or migrate missing fields before saving.
Useful? React with 👍 / 👎.
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/stores/accounts/accounts-actions.ts`:
- Around line 644-671: Ensure importAccount does not leave a persisted account
when importAccountHistory fails: validate the imported history before addAccount
or, preferably, catch history-import failures and remove the newly created
account and associated metadata before rethrowing. Anchor the cleanup to the
account created by accountsDatabase.addAccount and the subsequent
importAccountHistory call, preserving successful imports. Add a regression test
covering the failure path and verifying the account is rolled back.
In `@src/stores/accounts/accounts-database.ts`:
- Around line 880-894: Update importAccountHistory so replacing comments, votes,
and edits is failure-safe: preserve each existing store and restore all prior
data/metadata if any replaceDatabaseArray operation fails, rather than allowing
Promise.all to leave partial results. Also explicitly document the public
method’s unconditional replacement behavior or reject calls for accounts with
existing history, preserving history unless replacement is intentional.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 441fa259-45dc-4ef3-ab86-4870d8764a4c
📒 Files selected for processing (4)
src/stores/accounts/accounts-actions.test.tssrc/stores/accounts/accounts-actions.tssrc/stores/accounts/accounts-database.test.tssrc/stores/accounts/accounts-database.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4902118. Configure here.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Summary
Root cause
Account backups were replayed one record at a time through normal mutation paths. In particular, every imported edit rebuilt derived indexes and post summaries, making larger imports effectively quadratic. The flow also created an unused default account and started a refresh watcher for every imported comment.
Impact
A 100 KiB account file is not inherently excessive. In a production 5chan preview, representative 100 KiB imports improved as follows:
A mixed 100 KiB account imported in 90.7 ms normally and 507.9 ms under 4x CPU throttling with constrained networking. Import and re-export preserved all 100 comments, 100 votes, and 50 edits. No 5chan source change is required.
Verification
Note
Medium Risk
Changes touch local account identity, IndexedDB history writes, and import failure rollback; mistakes could corrupt or partially apply account data, though extensive new tests cover bulk import and rollback paths.
Overview
Account backup import no longer creates a disposable default account and replays each comment, vote, and edit through the normal single-record APIs. Imports merge
getDefaultAccountFields()with the backup, assign a new localid(so re-import cannot overwrite), hydrate once viaaddAccount(..., { returnHydratedAccount: true }), then persist history throughimportAccountHistoryin one bulk replace with snapshot rollback on failure; failed history import removes the new account and destroys its PKC client.importAccountHistorysanitizes comments/votes/edits, rebuilds vote and edit indexes and summaries once, and usesreplaceDatabaseArraysWithRollbackacross the three per-account history stores.Post-import, comment update watchers match startup: only the ten newest comments via
getInitAccountCommentsToUpdate, not the full history.addAccountgains stricter name-map handling (stale vacated names, missing metadata) and optional hydrated return without destroying the PKC instance.getDefaultAccountFieldsis shared between default account generation and import defaults.Reviewed by Cursor Bugbot for commit 7c2036f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes