perf(ui): incremental paged home timeline — reloads stop being O(wallet) - #976
Merged
Merged
Conversation
…llet) HomeViewModel's reload was the last O(wallet) hot path after the scoped- fetch work: every debounced sync/save tick re-materialized and re-wrapped the entire history (fetchAll -> walletTxRollup + full IN-fetch), measured 4.5-8.6s per tick on a 7,007-tx CoinJoin-heavy wallet. During a recovery sync that burned seconds of CPU per second, indefinitely. The feed now renders from a windowed row cache instead of the whole history: - First paint loads the newest ~100 rows via a firstSeen-index keyset page (fetchTimelineWindow). Pages are DAY-COMPLETED — the fetch always finishes its boundary calendar day, so a loaded day is never partially represented and the per-day CoinJoin mixing groups stay exact without a full-history pass. firstSeen doubles as the day key: the persister adopts the block timestamp once a tx is mined, so it matches the display date for settled history. - Scrolling to the feed's tail pages the next day-completed slice in (fetchOlderTimelinePage + a stamp-keyed sentinel row that re-fires while visible, so filters that match nothing keep paging). - Save/balance/sync ticks reconcile via fetchTimelineDelta: rows with lastUpdated past the reconcile stamp AND firstSeen inside the window, SQL-side. Restored history landing below the window costs nothing. Saves that DELETE feed rows (UnconfirmedTransactionRemover) and fiat- currency changes flag a window-sized refetch instead (deltas can't see deletions; cached wrappers hold currency-specific strings). - The Rewards/Masternode filter gates are answered by fetchLimit=1 existence probes on transactionTypeKind (coinbase=8, provider kinds 2-5) — never by scanning wrapped history. The cross-day "CoinJoin Withdrawals" group totals all tagged sweeps via point lookups (CoinJoinWithdrawalStore.allTxids) and renders once its day is paged in. Shielded/platform items clamp to the loaded day range; the "Date unknown" band renders only with full history loaded. - A recovery-sync growth cap trims an untouched full-history window back to ~one page past 400 rows (never after explicit paging). - Fixed in passing: UInt64 predicate bounds round-trip through SQLite's signed Int64, so a .max sentinel compared as -1 and matched nothing — scopedRows clamps bounds to Int64.max. TransactionSource grows the timeline API with fixture defaults (whole allTransactions as one complete window), so the onboarding stub and previews keep working unchanged. Measured on the synthetic 7,007-tx mainnet store (QA-iPhone16 sim, lldb in-process): first page 558ms cold / 173ms warm (100 rows), older page 310ms (104 rows), no-change delta 1ms, realistic 10-min delta 4ms (2 rows), filter-gate probes 12-30ms — vs 4.5-6.1s per full pass (idle; 8.6s under recovery-sync load). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 55 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesTimeline history management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WalletOwner
participant HomeView
participant HomeViewModel
participant TransactionSource
WalletOwner->>HomeView: scroll to history tail
HomeView->>HomeViewModel: loadMoreHistory()
HomeViewModel->>TransactionSource: request older timeline page
TransactionSource-->>HomeViewModel: return page and availability
HomeViewModel-->>HomeView: publish rows and page stamp
HomeView-->>WalletOwner: render additional history
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (1)
DashWallet/Sources/UI/Home/Views/HomeViewModel.swift (1)
687-691: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit
rebuildTimelineItems— SwiftLint reports a complexity error.SwiftLint reports
cyclomatic_complexity25 against the limit of 10 at Line 687, at error severity. The method now performs sweep-txid backfill, sorting, filtering, CoinJoin grouping, activity interleaving, grouping, and publishing.Extract the Core-row classification loop, the activity interleaving, and the publish step into separate private methods.
🤖 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 `@DashWallet/Sources/UI/Home/Views/HomeViewModel.swift` around lines 687 - 691, Reduce the cyclomatic complexity of rebuildTimelineItems by extracting its Core-row classification loop, activity interleaving, and publishing logic into separate private methods. Keep rebuildTimelineItems responsible for orchestration and preserve the existing ordering, filtering, grouping, and published output behavior.Source: Linters/SAST tools
🤖 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 `@DashWallet/Sources/UI/Home/Views/HomeView.swift`:
- Around line 404-419: Update the HomeView transaction-content layout so the
pagination sentinel controlled by canLoadMoreHistory is rendered even when
txItems is empty. Move or duplicate the existing ProgressView block outside the
txItems.isEmpty conditional, preserving its historyPageStamp identity and
loadMoreHistory onAppear behavior so filtered pagination continues.
In `@DashWallet/Sources/UI/Home/Views/HomeViewModel.swift`:
- Around line 556-564: In the pendingWindowRefetch branch, move the
pendingWindowRefetch = false assignment into the successful timelineDelta
handling after replaceWindow(with: delta) completes. Keep the flag set when
timelineDelta returns nil so the refetch request is retried later.
- Around line 622-637: The trim boundary in trimWindowIfNeeded() must use the
same firstSeen key as olderTimelinePage and timelineDelta. Sort and determine
the anchor day from each transaction’s firstSeen value, then retain rows based
on that value while preserving the existing dictionary rebuild,
windowOldestDayStart update, and hasOlderHistory behavior.
---
Nitpick comments:
In `@DashWallet/Sources/UI/Home/Views/HomeViewModel.swift`:
- Around line 687-691: Reduce the cyclomatic complexity of rebuildTimelineItems
by extracting its Core-row classification loop, activity interleaving, and
publishing logic into separate private methods. Keep rebuildTimelineItems
responsible for orchestration and preserve the existing ordering, filtering,
grouping, and published output behavior.
🪄 Autofix
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: 548d406b-2e0c-4b2c-8654-e2dedc5bea67
📒 Files selected for processing (3)
DashWallet/Sources/Models/CoinJoin/CoinJoinWithdrawalStore.swiftDashWallet/Sources/UI/Home/Views/HomeView.swiftDashWallet/Sources/UI/Home/Views/HomeViewModel.swift
5 tasks
…, firstSeen-keyed trim Review feedback on the paged home timeline (#976): - The tail sentinel now renders outside the empty/non-empty branch: a filter can hide every loaded row, and the empty state previously could not page deeper to find matches. The "no transactions to display" copy shows only when no unloaded history remains — with pages left it was a false claim. - `pendingWindowRefetch` clears only after the window re-read actually happens; a nil delta (host momentarily unbound) previously dropped the request and left currency-stale wrappers until an unrelated trigger. - The recovery growth cap re-tightens by refetching the first page from the store instead of trimming in memory: the store computes the boundary in firstSeen space — the same key paging and deltas filter by — while cached wrappers only expose the display date, which can differ for legacy rows and let a date-keyed trim drop rows paging never re-fetches. - rebuildTimelineItems split (input assembly, activity interleave, publish) to bring its cyclomatic complexity back under the SwiftLint bound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Note
Same change as #974, which was merged into
feat/marketplace-browseafter that branch had already been squash-merged to develop (#972) — so the paged timeline never reached develop. This is the identical commit rebased onto develop (clean rebase; the three touched files are byte-identical) and re-verified with the canonicaldashpaybuild on the develop base. If it instead lands via a futurefeat/marketplace-browsemerge, this PR can simply be closed.Issue being fixed or feature implemented
After the scoped wallet-source fetches (#972), HomeViewModel's full reload was the last O(wallet) hot path: every debounced sync/save tick called
fetchAll()→fetchAndWrap, re-materializing and re-wrapping the entire history — measured 4.5–6.1s idle (8.6s under recovery-sync load) per tick on a 7,007-tx CoinJoin-heavy wallet. During a recovery sync the reload ticks burned seconds of CPU each, indefinitely.What was done?
The feed now renders from a windowed row cache instead of the whole history:
fetchTimelineWindowloads the newest ~100 rows via afirstSeen-index keyset scan and always finishes its boundary calendar day, so a loaded day is never partially represented and the per-day CoinJoin mixing groups stay exact without a full-history pass.firstSeendoubles as the day key: the persister adopts the block timestamp once a tx is mined, so it equals the display date for settled history.fetchOlderTimelinePage), auto-continuing while visible so a filter that matches nothing keeps paging.fetchTimelineDeltafetches only rows withlastUpdatedpast the reconcile stamp ANDfirstSeeninside the window, SQL-side. Restored history landing below the window costs nothing. Saves that delete feed rows (UnconfirmedTransactionRemover) and fiat-currency changes flag a window-sized refetch instead — deltas can't observe deletions, and cached wrappers hold currency-specific strings.fetchLimit=1existence probes ontransactionTypeKind(coinbase=8, provider kinds 2–5); the cross-day "CoinJoin Withdrawals" group totals every tagged sweep via point lookups (CoinJoinWithdrawalStore.allTxids) and renders once its day is paged in; shielded/platform items clamp to the loaded day range; the "Date unknown" band renders only with full history loaded.TransactionSourcegrows the timeline API with fixture defaults (wholeallTransactionsas one complete window), so the onboarding stub and previews work unchanged.#Predicateround-trip through SQLite's signed Int64 — a.max"no upper bound" sentinel compared as −1 and silently matched nothing.scopedRowsclamps bounds toInt64.max.How Has This Been Tested?
dashpaysimulator build (ARCHS=arm64) on the develop base after the rebase.hasRewards/hasMasternodesboth true on the synthetic store).Reviewer notes: the delta stamp must never advance from older-page fetches (a paged-in row's
lastUpdatedcan postdate window updates the next delta still has to pick up); receipt matching for shielded items sees the loaded window — the only miss is a receipt up to 1h before an item across the window's bottom midnight, which self-resolves when that day pages in.Breaking Changes
None — UI behavior change only: history below the loaded window renders progressively as the user scrolls instead of all at once.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit