perf(accounts): slim account history storage and query hot paths#42
Merged
tomcasaburi merged 4 commits intomasterfrom Mar 20, 2026
Merged
perf(accounts): slim account history storage and query hot paths#42tomcasaburi merged 4 commits intomasterfrom
tomcasaburi merged 4 commits intomasterfrom
Conversation
Preserve full backup compatibility while reducing account history bloat and lookup overhead.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds per-account comment indexes and edit summaries, sanitizes persisted account history, introduces lazy-loading for account edits, extends comment/vote hook query APIs (CID/index scoping, pagination, sorting, timestamp filters), and updates store/database/actions to maintain new indexes and summaries. Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as useAccountEdits Hook
participant Store as Accounts Store
participant Actions as Actions/Internal
participant DB as Database Layer
participant Cache as EditLoad Cache
Hook->>Store: read accountsEdits[accountId] (hot)
alt accountsEditsLoaded[accountId] is false
Hook->>Actions: ensureAccountEditsLoaded(accountId)
Actions->>Cache: check accountEditsLoadPromises[accountId]
alt promise exists
Cache-->>Actions: return pending promise
else
Actions->>DB: getAccountEdits(accountId) (rebuild __targetToIndices/__summary)
DB-->>Actions: edits + summaries
Actions->>Store: merge edits, set accountsEditsSummaries, accountsEditsLoaded=true
Actions->>Cache: resolve promise
end
Actions-->>Hook: notify succeeded
end
Store-->>Hook: return edits or summaries
sequenceDiagram
participant Hook as useAccountComments Hook
participant Store as Accounts Store
participant Utils as IndexUtils
Hook->>Store: request comments with options {commentCid?, commentIndices?, communityAddress?, parentCid?, newerThan?, page?, pageSize?, sortType?}
Store->>Utils: lookup indices via accountsCommentsIndexes[accountId]
Utils-->>Store: matched indices (by community/parent/indices/CID)
Store->>Store: apply newerThan filter
Store->>Store: apply sortType / order
Store->>Store: apply page/pageSize slice
Store-->>Hook: return filtered, sorted, paginated comment list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
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)
📝 Coding Plan
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/accounts/accounts.ts (1)
559-599:⚠️ Potential issue | 🟠 MajorSort votes by timestamp before applying
orderand pagination.This array is built from a
commentCidmap, so its default order is object key order, not vote time. Re-voting an oldercommentCidkeeps its original slot, which makes the new “recent votes” and pagedorder: "desc"queries return the wrong items.⏱️ Suggested fix
if (filter) { accountVotesArray = accountVotesArray.filter(filter); } + accountVotesArray = [...accountVotesArray].sort( + (a, b) => (a.timestamp || 0) - (b.timestamp || 0), + ); if (order === "desc") { accountVotesArray.reverse(); } if (typeof pageSize === "number" && pageSize > 0) { const pageNumber = Math.max(page || 0, 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/accounts/accounts.ts` around lines 559 - 599, The array built in filteredAccountVotesArray should be sorted by vote timestamp before applying order and pagination: after you finish applying filters to accountVotesArray (but before the order === "desc" and pageSize slicing), call a deterministic sort on timestamp (e.g., accountVotesArray.sort((a, b) => a.timestamp - b.timestamp)) so recent/older ordering is correct; keep the existing order === "desc" reverse behavior (or alternatively sort by b.timestamp - a.timestamp when order === "desc") and then apply pagination using page and pageSize as currently implemented.
🧹 Nitpick comments (4)
src/stores/accounts/accounts-store.ts (1)
104-125: Add a note for the hot/cold edit split.Preloading
accountsEditsSummarieswhile intentionally initializingaccountsEditsto empty objects is the key behavior change here, but it reads like an accidental omission without a short comment. A one-liner explaining that full edit history stays cold and is lazy-loaded would make this much easier to maintain.As per coding guidelines, "Add comments for complex/non-obvious code; skip obvious comments."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-store.ts` around lines 104 - 125, Add a one-line explanatory comment above the preloading/initialization block clarifying the hot/cold edit split: note that accountsEditsSummaries is eagerly preloaded (hot) while accountsEdits is intentionally initialized to empty objects and loaded lazily (cold) to avoid fetching full edit histories; reference the accountsEditsSummaries, accountsEdits, and accountsEditsLoaded symbols so future readers understand this is intentional and not an omission.src/stores/accounts/accounts-actions-internal.ts (1)
171-180: Reindex only the mutated account here.Both call sites hand
getAccountsCommentsIndexes(...)the fullAccountsCommentsmap just to replace one bucket. On large local histories, every single comment update/CID resolution becomes a full reindex.♻️ Suggested narrowing
- accountsStore.setState(({ accountsComments }) => { + accountsStore.setState(({ accountsComments, accountsCommentsIndexes }) => { // account no longer exists if (!accountsComments[account.id]) { log.error( `startUpdatingAccountCommentOnCommentUpdateEvents comment.on('update') invalid accountsStore.accountsComments['${account.id}'] '${ accountsComments[account.id] }', account may have been deleted`, ); return {}; } const updatedAccountComments = [...accountsComments[account.id]]; const previousComment = updatedAccountComments[currentIndex]; const updatedAccountComment = utils.clone({ ...storedUpdatedComment, index: currentIndex, accountId: account.id, }); updatedAccountComments[currentIndex] = updatedAccountComment; const nextAccountsComments = { ...accountsComments, [account.id]: updatedAccountComments, }; + const nextAccountCommentsIndexes = + getAccountsCommentsIndexes({ [account.id]: updatedAccountComments } as AccountsComments)[ + account.id + ]; return { accountsComments: nextAccountsComments, accountsCommentsIndexes: { - ...accountsStore.getState().accountsCommentsIndexes, - [account.id]: getAccountsCommentsIndexes(nextAccountsComments)[account.id], + ...accountsCommentsIndexes, + [account.id]: nextAccountCommentsIndexes, }, }; });Apply the same narrowing in
addCidToAccountComment(...)below.Also applies to: 292-296
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-actions-internal.ts` around lines 171 - 180, The code reindexes the entire AccountsComments map by calling getAccountsCommentsIndexes(nextAccountsComments) when only a single account bucket changed; change this to reindex only the mutated account by computing the index for account.id from the updatedAccountComments (or by calling getAccountsCommentsIndexes with a map containing just [account.id]: updatedAccountComments) and merge that single-entry result into accountsCommentsIndexes; apply the same change in addCidToAccountComment(...) so both locations avoid full-map reindexing.src/stores/accounts/accounts-actions.test.ts (1)
1670-1690: Assert the summary bucket in this moderation-path test too.This branch only verifies
accountsEdits, butuseEditedComment()can now serve fromaccountsEditsSummariesbefore lazy edit hydration runs. A broken summary write would still pass here.🧪 Suggested assertion
expect(accountsStore.getState().accountsEdits[account.id].cid).toHaveLength(1); + expect(accountsStore.getState().accountsEditsSummaries[account.id]?.cid).toBeDefined();Based on learnings: Maintain mandatory 100% test coverage for hooks and stores (
src/hooks/,src/stores/); every feature or bug fix must include/adjust tests to keep coverage at 100%, and when changing hooks or stores, consider whether existing tests cover the change; if not, add a test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-actions.test.ts` around lines 1670 - 1690, The test for publishCommentModeration only asserts accountsEdits but must also assert the summary bucket is initialized; update the test around accountsActions.publishCommentModeration to also check accountsStore.getState().accountsEditsSummaries[account.id] (or the summary structure used by useEditedComment) has been created and contains the expected cid entry (e.g., .cid length equals 1) after the call so broken summary writes fail the test; locate the assertion area in the test and add the corresponding expect on accountsEditsSummaries for the same account.id.src/stores/accounts/accounts-actions.ts (1)
130-229: Extract the edit-summary reducers into a dedicated module.This adds another chunk of reducer logic to a store action file that is already far past the repo’s ~300-line guidance, and it overlaps with summary logic that
src/stores/accounts/utils.tsalready owns viagetAccountEditPropertySummary(). Moving these helpers next to the existing account-edit utilities would keep the action layer thinner and reduce drift.As per coding guidelines, "Keep files focused; split large stores or hooks when they exceed ~300 lines".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-actions.ts` around lines 130 - 229, The reducer/summary helpers (doesStoredAccountEditMatch, sanitizeStoredAccountEdit, accountEditNonPropertyNames, addStoredAccountEditSummaryToState, removeStoredAccountEditSummaryFromState) should be extracted out of the actions file and moved into the accounts utils module alongside getAccountEditPropertySummary; inside that utils module, reuse getAccountEditPropertySummary where appropriate, keep the same exported function names, and update all imports/usages in the actions file to import these helpers from the utils module so the action file shrinks and summary logic is centralized.
🤖 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/hooks/accounts/accounts.test.ts`:
- Around line 1323-1346: The test "useAccountEdits logs and keeps initializing
when lazy load rejects" mutates store state but only restores
accountsActionsInternal; capture the original accountsEditsLoaded value for the
active accountId (from store.default.getState().accountsEditsLoaded[accountId])
before setting it to false and then restore it in the finally block along with
accountsActionsInternal (i.e., setState with both accountsActionsInternal:
internal and accountsEditsLoaded: originalEditsLoaded or by restoring the
specific key [accountId] to its previous boolean) so the shared store is
returned to its prior state after the test.
In `@src/hooks/accounts/accounts.ts`:
- Around line 499-518: The hook useAccountComment currently reads
accountComment.state directly from the raw accountsComments entry so
published/pending comments fall back to "initializing"; restore the derived
status the same way useAccountComments does by synthesizing state when building
accountComment (use the resolvedCommentIndex / accountComments lookup used here)
rather than trusting accountComment.state — either call the existing helper that
computes comment state or replicate that logic at the end of the useMemo so
accountComment includes the correct derived state (e.g., published vs pending vs
initializing).
In `@src/stores/accounts/accounts-actions.ts`:
- Around line 572-577: The store update is incorrectly marking
accountsEditsLoaded true after an optimistic append, which can hide missing
historical edits; in the update logic that sets accountsEdits,
accountsEditsSummaries and accountsEditsLoaded (referenced as accountsEdits,
accountsEditsSummaries, accountsEditsLoaded in this diff and the same pattern
around the other occurrences), do not set accountsEditsLoaded to true after an
optimistic/local append—leave it false until a real load/merge completes or
perform a proper merge/hydration of the existing edits bucket before flipping
the flag so lazy loaders will trigger a DB read.
- Around line 897-917: saveCreatedAccountComment currently always writes using
the captured accountCommentIndex which can be stale after deleteComment
reindexes/abandons a publish; change the function to read the session's live
index before persisting (e.g. pull currentIndex from
accountsStore.getState().accountsCommentsIndexes[account.id] or compute via
getAccountCommentsIndex on the live comments array) and: if the live index is
undefined/marked-abandoned then no-op and do not call
accountsDatabase.addAccountComment; otherwise use that live index instead of the
captured accountCommentIndex when calling accountsDatabase.addAccountComment and
when writing state (keep savedOnce semantics but update to reflect successful
persistence against the live index). Ensure the same live-index is used for both
the DB call (accountsDatabase.addAccountComment) and the subsequent
accountsStore.setState update to avoid overwriting the wrong slot.
- Around line 199-229: The function removeStoredAccountEditSummaryFromState
currently computes nextTargetSummary from the pre-removal accountsEdits bucket,
so a rolled-back/failed edit can remain the summary; instead, build the
editsForTarget as the accountsEdits[accountId]?.[editTarget] list with the
storedAccountEdit removed (filtering by a unique identifier present on
storedAccountEdit such as txHash/commentCid/cid or strict object inequality),
then pass that filtered list into getAccountEditPropertySummary to derive
nextTargetSummary and continue updating nextAccountSummary as before
(references: removeStoredAccountEditSummaryFromState, accountsEdits, accountId,
storedAccountEdit, getAccountEditPropertySummary, editTarget).
- Around line 152-229: Tests are missing for the new summary/lazy-hydration and
rollback/publish branches affecting store behavior; add unit tests that exercise
addStoredAccountEditSummaryToState and removeStoredAccountEditSummaryFromState
to verify summary add/remove semantics (including commentModeration
normalization), cover rollback after terminal challenge verification by
simulating edits and ensuring summaries update on rollback, and validate
accountsEditsLoaded behavior for imported accounts by initializing accountsEdits
and accountsEditsSummaries and asserting lazy-hydration populates expected
summaries via getAccountEditPropertySummary; include cases for imported
accounts, timestamp ordering, and deletion of empty targets.
- Around line 385-409: The deletion sequence removes per-account structures but
leaves commentCidsToAccountsComments stale; update the code that prepares state
for accountsStore.setState to also rebuild commentCidsToAccountsComments (or
remove any entries referencing account.id) — e.g., after creating
newAccountsComments (and before calling accountsStore.setState), iterate
newAccountsComments to reconstruct commentCidsToAccountsComments from the
remaining accounts' comment arrays (or filter existing
commentCidsToAccountsComments to drop mappings for account.id) so CID-based
lookups no longer return the deleted account.
In `@src/stores/accounts/accounts-database.ts`:
- Around line 593-595: getAccountEditTarget now returns
communityAddress/subplebbitAddress as valid targets, but the
persistence/rollback guards still require edit.commentCid so community edits
aren't persisted or removable; update the persistence checks in the account
edits save/remove code paths (the functions that currently check
edit.commentCid) to instead call and validate getAccountEditTarget(edit) (or
check for any of edit.commentCid || edit.communityAddress ||
edit.subplebbitAddress) before proceeding, and use that resolved target when
constructing keys or performing lookups so community edits are persisted and can
be rolled back/removed.
In `@src/stores/accounts/utils.ts`:
- Around line 67-99: The function sanitizeStoredAccountComment currently
deep-clones the entire Comment via cloneWithoutFunctions(...) then deletes
replies.pages, which still forces cloning the expensive replies.pages subtree;
instead, remove or replace comment.replies.pages before calling
cloneWithoutFunctions so the heavy subtree is never traversed. Concretely,
create a shallow preprocessedComment from the input comment that omits
replies.pages (e.g., copy comment and, if comment.replies?.pages exists, copy
replies without pages or set replies.pages = undefined) and pass that
preprocessed object into cloneWithoutFunctions; keep the existing post-clone
cleanup logic for sanitizedComment.replies as-is.
---
Outside diff comments:
In `@src/hooks/accounts/accounts.ts`:
- Around line 559-599: The array built in filteredAccountVotesArray should be
sorted by vote timestamp before applying order and pagination: after you finish
applying filters to accountVotesArray (but before the order === "desc" and
pageSize slicing), call a deterministic sort on timestamp (e.g.,
accountVotesArray.sort((a, b) => a.timestamp - b.timestamp)) so recent/older
ordering is correct; keep the existing order === "desc" reverse behavior (or
alternatively sort by b.timestamp - a.timestamp when order === "desc") and then
apply pagination using page and pageSize as currently implemented.
---
Nitpick comments:
In `@src/stores/accounts/accounts-actions-internal.ts`:
- Around line 171-180: The code reindexes the entire AccountsComments map by
calling getAccountsCommentsIndexes(nextAccountsComments) when only a single
account bucket changed; change this to reindex only the mutated account by
computing the index for account.id from the updatedAccountComments (or by
calling getAccountsCommentsIndexes with a map containing just [account.id]:
updatedAccountComments) and merge that single-entry result into
accountsCommentsIndexes; apply the same change in addCidToAccountComment(...) so
both locations avoid full-map reindexing.
In `@src/stores/accounts/accounts-actions.test.ts`:
- Around line 1670-1690: The test for publishCommentModeration only asserts
accountsEdits but must also assert the summary bucket is initialized; update the
test around accountsActions.publishCommentModeration to also check
accountsStore.getState().accountsEditsSummaries[account.id] (or the summary
structure used by useEditedComment) has been created and contains the expected
cid entry (e.g., .cid length equals 1) after the call so broken summary writes
fail the test; locate the assertion area in the test and add the corresponding
expect on accountsEditsSummaries for the same account.id.
In `@src/stores/accounts/accounts-actions.ts`:
- Around line 130-229: The reducer/summary helpers (doesStoredAccountEditMatch,
sanitizeStoredAccountEdit, accountEditNonPropertyNames,
addStoredAccountEditSummaryToState, removeStoredAccountEditSummaryFromState)
should be extracted out of the actions file and moved into the accounts utils
module alongside getAccountEditPropertySummary; inside that utils module, reuse
getAccountEditPropertySummary where appropriate, keep the same exported function
names, and update all imports/usages in the actions file to import these helpers
from the utils module so the action file shrinks and summary logic is
centralized.
In `@src/stores/accounts/accounts-store.ts`:
- Around line 104-125: Add a one-line explanatory comment above the
preloading/initialization block clarifying the hot/cold edit split: note that
accountsEditsSummaries is eagerly preloaded (hot) while accountsEdits is
intentionally initialized to empty objects and loaded lazily (cold) to avoid
fetching full edit histories; reference the accountsEditsSummaries,
accountsEdits, and accountsEditsLoaded symbols so future readers understand this
is intentional and not an omission.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0502053-980a-4503-a41b-d48d72081828
📒 Files selected for processing (13)
README.mdsrc/hooks/accounts/accounts.test.tssrc/hooks/accounts/accounts.tssrc/stores/accounts/accounts-actions-internal.tssrc/stores/accounts/accounts-actions.test.tssrc/stores/accounts/accounts-actions.tssrc/stores/accounts/accounts-database.test.tssrc/stores/accounts/accounts-database.tssrc/stores/accounts/accounts-store.test.tssrc/stores/accounts/accounts-store.tssrc/stores/accounts/utils.test.tssrc/stores/accounts/utils.tssrc/types.ts
Comment on lines
+152
to
+229
| export const addStoredAccountEditSummaryToState = ( | ||
| accountsEditsSummaries: Record<string, Record<string, any>>, | ||
| accountId: string, | ||
| storedAccountEdit: any, | ||
| ) => { | ||
| const editTarget = | ||
| storedAccountEdit.commentCid || | ||
| storedAccountEdit.communityAddress || | ||
| storedAccountEdit.subplebbitAddress; | ||
| if (!editTarget) { | ||
| return { accountsEditsSummaries }; | ||
| } | ||
|
|
||
| const accountEditsSummary = accountsEditsSummaries[accountId] || {}; | ||
| const targetSummary = accountEditsSummary[editTarget] || {}; | ||
| const nextSummary = { ...targetSummary }; | ||
| const normalizedEdit = storedAccountEdit.commentModeration | ||
| ? { ...storedAccountEdit, ...storedAccountEdit.commentModeration, commentModeration: undefined } | ||
| : storedAccountEdit; | ||
|
|
||
| for (const propertyName in normalizedEdit) { | ||
| if ( | ||
| normalizedEdit[propertyName] === undefined || | ||
| accountEditNonPropertyNames.has(propertyName) | ||
| ) { | ||
| continue; | ||
| } | ||
| const previousTimestamp = nextSummary[propertyName]?.timestamp || 0; | ||
| if ((normalizedEdit.timestamp || 0) >= previousTimestamp) { | ||
| nextSummary[propertyName] = { | ||
| timestamp: normalizedEdit.timestamp, | ||
| value: normalizedEdit[propertyName], | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| accountsEditsSummaries: { | ||
| ...accountsEditsSummaries, | ||
| [accountId]: { | ||
| ...accountEditsSummary, | ||
| [editTarget]: nextSummary, | ||
| }, | ||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| export const removeStoredAccountEditSummaryFromState = ( | ||
| accountsEditsSummaries: Record<string, Record<string, any>>, | ||
| accountsEdits: Record<string, Record<string, any[]>>, | ||
| accountId: string, | ||
| storedAccountEdit: any, | ||
| ) => { | ||
| const editTarget = | ||
| storedAccountEdit.commentCid || | ||
| storedAccountEdit.communityAddress || | ||
| storedAccountEdit.subplebbitAddress; | ||
| if (!editTarget) { | ||
| return { accountsEditsSummaries }; | ||
| } | ||
|
|
||
| const nextTargetSummary = getAccountEditPropertySummary( | ||
| accountsEdits[accountId]?.[editTarget] || [], | ||
| ); | ||
| const nextAccountSummary = { ...(accountsEditsSummaries[accountId] || {}) }; | ||
| if (Object.keys(nextTargetSummary).length > 0) { | ||
| nextAccountSummary[editTarget] = nextTargetSummary; | ||
| } else { | ||
| delete nextAccountSummary[editTarget]; | ||
| } | ||
|
|
||
| return { | ||
| accountsEditsSummaries: { | ||
| ...accountsEditsSummaries, | ||
| [accountId]: nextAccountSummary, | ||
| }, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Add store tests for the new summary and lazy-hydration branches.
This change adds new exported reducers, import-time lazy edit initialization, and rollback/publish branches in a src/stores/ file, but no matching tests were included. Please cover at least summary add/remove behavior, rollback after terminal challenge verification, and accountsEditsLoaded behavior for imported accounts.
As per coding guidelines, "Maintain mandatory 100% test coverage for hooks and stores (src/hooks/, src/stores/); every feature or bug fix must include/adjust tests to keep coverage at 100%" and "When changing hooks or stores, consider whether existing tests cover the change; if not, add a test".
Also applies to: 544-580, 1281-1488
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/accounts/accounts-actions.ts` around lines 152 - 229, Tests are
missing for the new summary/lazy-hydration and rollback/publish branches
affecting store behavior; add unit tests that exercise
addStoredAccountEditSummaryToState and removeStoredAccountEditSummaryFromState
to verify summary add/remove semantics (including commentModeration
normalization), cover rollback after terminal challenge verification by
simulating edits and ensuring summaries update on rollback, and validate
accountsEditsLoaded behavior for imported accounts by initializing accountsEdits
and accountsEditsSummaries and asserting lazy-hydration populates expected
summaries via getAccountEditPropertySummary; include cases for imported
accounts, timestamp ordering, and deletion of empty targets.
Member
Author
|
Addressed the valid account-history review findings in the latest commits ( Local verification on the updated branch: |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/stores/accounts/accounts-actions.ts (1)
50-240: Extract the new publish-session and edit-summary helpers into dedicated modules.This PR adds two more non-trivial helper subsystems to an already very large store file. Pulling them out would make the publish/delete race handling and edit-summary rules much easier to test and reason about.
As per coding guidelines, "Keep files focused; split large stores or hooks when they exceed ~300 lines".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/accounts/accounts-actions.ts` around lines 50 - 240, Split the large accounts-actions file by extracting the publish-session logic (activePublishSessions, abandonedPublishKeys, getPublishSessionKey, registerPublishSession, abandonAndStopPublishSession, isPublishSessionAbandoned, getPublishSessionByIndex, getPublishSessionForComment, shiftPublishSessionIndicesAfterDelete, cleanupPublishSessionOnTerminal) into a new module (e.g., publishSessions) exporting those functions and the Map/Set accessors, and extract the edit-summary helpers (accountEditNonPropertyNames, getStoredAccountEditTarget, addStoredAccountEditSummaryToState, removeStoredAccountEditSummaryFromState, sanitizeStoredAccountEdit, doesStoredAccountEditMatch, getAccountEditPropertySummary if referenced) into a separate module (e.g., editSummaries) exporting the functions; update the original file to import these symbols and replace local definitions with the imports, adjust any internal references (e.g., maybeUpdateAccountComment still in store) and ensure all callers compile, then add unit tests for publish-session race behaviors and edit-summary rules in their new modules.
🤖 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-actions-internal.ts`:
- Around line 384-390: The load handler for loadPromise is replacing the
per-account edits bucket and clobbering optimistic local edits; instead of
setting accountsEdits: { ...accountsEdits, [accountId]: accountEdits } replace
that assignment in accountsStore.setState with a merge of the existing bucket
and the fetched snapshot so local optimistic entries from publishCommentEdit /
publishCommentModeration are preserved. Concretely, inside the then(...) for
accountsDatabase.getAccountEdits(accountId) read the current
accountsEdits[accountId] from the state, produce a merged list/map that unions
items by edit id (or uses timestamp to keep the newer entry), and write
accountsEdits: { ...accountsEdits, [accountId]: mergedBucket } while still
setting accountsEditsLoaded[accountId] = true.
In `@src/stores/accounts/accounts-actions.ts`:
- Around line 58-65: The publish session Map is using a mutable array index as
the key which can be reused after deletes and cause sessions to be overwritten;
change to a stable, immutable session identifier: update registerPublishSession
to generate or accept a stable sessionId (e.g., a UUID or a composite of
accountId + originalCommentId) and use that sessionId as the Map key instead of
index, store the original index inside the session object (currentIndex) for
position tracking, and update all usages that call getPublishSessionKey,
activePublishSessions.get/set/delete, and any cleanup logic (e.g., the code
referenced around registerPublishSession and the block at 121-128) to operate on
the new stable sessionId rather than the mutable index so sessions cannot be
accidentally reused.
In `@src/stores/accounts/accounts-database.ts`:
- Around line 621-641: The code is compacting sparse edit slots by calling
getDatabaseAsArray(...).filter(Boolean) which renumbers in-memory indices and
corrupts the mapping persisted via persistAccountEditsIndexes (used later by
getAccountEdits). In ensureAccountEditsDatabaseLayout, stop calling
.filter(Boolean); keep the array with holes so original numeric indices are
preserved when computing duplicateKeysToDelete and when calling
persistAccountEditsIndexes, or alternatively perform a full rewrite that renames
numeric DB keys to consecutive indices and update the DB before persisting
editsTargetToIndicesKey; update both the block in
ensureAccountEditsDatabaseLayout (using getAccountEditsDatabase,
getDatabaseAsArray, duplicateKeysToDelete, persistAccountEditsIndexes) and the
similar later block so numeric positions remain consistent with stored indices.
---
Nitpick comments:
In `@src/stores/accounts/accounts-actions.ts`:
- Around line 50-240: Split the large accounts-actions file by extracting the
publish-session logic (activePublishSessions, abandonedPublishKeys,
getPublishSessionKey, registerPublishSession, abandonAndStopPublishSession,
isPublishSessionAbandoned, getPublishSessionByIndex,
getPublishSessionForComment, shiftPublishSessionIndicesAfterDelete,
cleanupPublishSessionOnTerminal) into a new module (e.g., publishSessions)
exporting those functions and the Map/Set accessors, and extract the
edit-summary helpers (accountEditNonPropertyNames, getStoredAccountEditTarget,
addStoredAccountEditSummaryToState, removeStoredAccountEditSummaryFromState,
sanitizeStoredAccountEdit, doesStoredAccountEditMatch,
getAccountEditPropertySummary if referenced) into a separate module (e.g.,
editSummaries) exporting the functions; update the original file to import these
symbols and replace local definitions with the imports, adjust any internal
references (e.g., maybeUpdateAccountComment still in store) and ensure all
callers compile, then add unit tests for publish-session race behaviors and
edit-summary rules in their new modules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efbd8c6a-7a8a-482a-ba38-8930820b9d91
📒 Files selected for processing (11)
README.mdsrc/hooks/accounts/accounts.test.tssrc/hooks/accounts/accounts.tssrc/stores/accounts/accounts-actions-internal.tssrc/stores/accounts/accounts-actions.test.tssrc/stores/accounts/accounts-actions.tssrc/stores/accounts/accounts-database.test.tssrc/stores/accounts/accounts-database.tssrc/stores/accounts/accounts-store.tssrc/stores/accounts/utils.tssrc/types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/stores/accounts/accounts-store.ts
- src/stores/accounts/accounts-actions.test.ts
- src/hooks/accounts/accounts.test.ts
- src/hooks/accounts/accounts.ts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| }; | ||
| const state = storedAccountComment | ||
| ? getAccountCommentsStates([storedAccountComment])[0] | ||
| : "initializing"; |
There was a problem hiding this comment.
useAccountComment loses periodic pending-to-failed state recheck
Medium Severity
The refactored useAccountComment computes state directly via getAccountCommentsStates at render time but no longer goes through useAccountComments, which had a useInterval rechecking states every 60 seconds. Without that interval, a pending comment will never automatically transition to "failed" after the 20-minute timeout because nothing triggers a re-render when only wall-clock time has changed.
Additional Locations (1)
| removeStoredAccountEditFromState(accountsEdits, account.id, storedCommentEdit), | ||
| ); | ||
| return nextState; | ||
| }), |
There was a problem hiding this comment.
Rollback recalculates edit summary from incomplete cold state
Medium Severity
removeStoredAccountEditSummaryFromState recalculates the target summary from accountsEdits, which is incomplete in cold/lazy state. Rolling back a failed edit wipes the summary entry for that target, losing information from older edits that still exist in the database but haven't been lazily loaded yet. Similarly, useEditedComment prefers getAccountEditPropertySummary(commentEdits) over the more complete commentEditSummary whenever commentEdits?.length > 0, which can be true from optimistic additions alone.
Additional Locations (1)
| const doesStoredAccountEditMatch = (storedAccountEdit: any, targetStoredAccountEdit: any) => | ||
| storedAccountEdit?.clientId && targetStoredAccountEdit?.clientId | ||
| ? storedAccountEdit.clientId === targetStoredAccountEdit.clientId | ||
| : isEqual(storedAccountEdit, targetStoredAccountEdit); |
There was a problem hiding this comment.
Triplicated doesStoredAccountEditMatch across three store files
Low Severity
The identical doesStoredAccountEditMatch function is independently defined in accounts-actions-internal.ts, accounts-actions.ts, and accounts-database.ts. If the matching logic needs updating (e.g., adding a new identity field), all three copies must be kept in sync — a likely source of future inconsistencies. One canonical export from a shared utility would eliminate the risk.
Additional Locations (2)
This was referenced Mar 20, 2026
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.
Reduce account-history bloat by storing leaner account comments, deduplicating vote/edit persistence, and adding non-breaking query paths for hot account lookups. This keeps full account export/import behavior intact while letting apps avoid the worst local scans.
Closes #41
Note
Medium Risk
Touches persistence/migration logic for account comments, votes, and edits (including new indexes and lazy hydration), so regressions could affect local history integrity or query correctness across upgrades.
Overview
Reduces account-history storage bloat by sanitizing persisted account comments (dropping functions/sensitive fields and heavy
replies.pages) and introducing compact, indexed persistence for votes/edits with migration/repair for legacy and partially-written DB states.Expands
useAccountComments,useAccountVotes, anduseAccountCommentwith non-breaking query helpers (bycommentCid,commentIndices,communityAddress,parentCid,vote,newerThan, pagination, andsortType/deprecatedorder) and adds per-account comment indexes to avoid full scans.Keeps edit history cold by default: store init loads
accountsEditsSummarieswhileaccountsEditsare lazily hydrated viaensureAccountEditsLoaded, anduseEditedCommentnow prefers summaries when full edits aren’t loaded. Publish-session tracking for pending comments is hardened to prevent deleted/reindexed pending items from being resurrected or receiving later event updates.Written by Cursor Bugbot for commit 7feb65d. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests