Skip to content

fix: improve bank sync UX with accurate queue counts#6474

Closed
robertlestak wants to merge 40 commits intoactualbudget:masterfrom
robertlestak:feature/sync-queue
Closed

fix: improve bank sync UX with accurate queue counts#6474
robertlestak wants to merge 40 commits intoactualbudget:masterfrom
robertlestak:feature/sync-queue

Conversation

@robertlestak
Copy link

@robertlestak robertlestak commented Dec 22, 2025

Summary

This PR fixes UX issues with the bank sync queue system, specifically addressing inaccurate notification counts when syncing multiple accounts.

Changes Made

1. Fixed Notification Banner Count Logic

  • Problem: The top notification banner always showed "1 account remaining" regardless of how many accounts were queued for sync
  • Solution: Implemented smart counting logic in BankSyncStatus.tsx:
    • For individual accounts: displays total queued requests (1, 2, 3...)
    • For 'Sync All': displays total accounts being synced

Technical Details

  • Modified BankSyncStatus.tsx to use conditional counting logic based on presence of 'ALL_ACCOUNTS' queue requests
  • Updated SimpleFin batch processing in accountsSlice.ts to progressively update the accountsSyncing array

Testing

  • Manually tested individual account syncing with multiple rapid clicks
  • Manually tested 'Sync All' functionality with 8 SimpleFin accounts
  • Verified notification counts update correctly in real-time
  • Confirmed button styling consistency across desktop and mobile interfaces

Screenshots/Demo

Screenshot 2025-12-22 at 11 15 07

The notification banner now correctly shows:

  • Individual accounts: "Syncing... 1 account remaining", "Syncing... 2 accounts remaining", etc.
  • Sync All: "Syncing... 8 accounts remaining" (total accounts being processed)

Fixes the issue where counts were always showing "1 account remaining" regardless of actual queue size.


Note: This PR was created with AI assistance and should be labeled accordingly.

Summary by CodeRabbit

  • New Features

    • "Sync All" added on desktop and mobile with a new hook exposing linked accounts and a trigger.
  • Improvements

    • Sync requests are now queued and processed sequentially; UI shows accurate "Syncing... X account(s) remaining" and disables controls while active.
    • Progress updates are more reliable across batch and per-account syncs; status rendering simplified.
  • Bug Fixes

    • Per-item errors no longer block the queue; processing continues and recovers gracefully.

✏️ Tip: You can customize this high-level summary in your review settings.


Bundle Stats

Bundle Files count Total bundle size % Changed
desktop-client 27 14.65 MB → 14.66 MB (+3.49 kB) +0.02%
loot-core 1 5.86 MB 0%
api 1 4.4 MB 0%
View detailed bundle stats

desktop-client

Total

Files count Total bundle size % Changed
27 14.65 MB → 14.66 MB (+3.49 kB) +0.02%
Changeset
File Δ Size
src/accounts/accountsSlice.ts 📈 +3.55 kB (+31.90%) 11.13 kB → 14.69 kB
src/components/mobile/accounts/AccountPage.tsx 📈 +206 B (+2.03%) 9.93 kB → 10.13 kB
src/components/mobile/accounts/AccountsPage.tsx 📈 +365 B (+1.66%) 21.43 kB → 21.79 kB
src/components/accounts/Account.tsx 📈 +399 B (+0.80%) 48.75 kB → 49.14 kB
src/components/transactions/SelectedTransactionsButton.tsx 📈 +4 B (+0.02%) 18.68 kB → 18.69 kB
src/components/BankSyncStatus.tsx 📉 -30 B (-1.24%) 2.36 kB → 2.33 kB
src/components/sidebar/Accounts.tsx 📉 -1005 B (-11.60%) 8.46 kB → 7.48 kB
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger

Asset File Size % Changed
static/js/index.js 9.49 MB → 9.49 MB (+2.54 kB) +0.03%
static/js/narrow.js 638.5 kB → 639.05 kB (+571 B) +0.09%
static/js/wide.js 164.05 kB → 164.44 kB (+403 B) +0.24%

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
static/js/indexeddb-main-thread-worker-e59fee74.js 12.94 kB 0%
static/js/workbox-window.prod.es5.js 5.64 kB 0%
static/js/da.js 106.62 kB 0%
static/js/de.js 180.44 kB 0%
static/js/en-GB.js 7.18 kB 0%
static/js/en.js 164.99 kB 0%
static/js/es.js 173.83 kB 0%
static/js/fr.js 179.97 kB 0%
static/js/it.js 171.44 kB 0%
static/js/nb-NO.js 157.23 kB 0%
static/js/nl.js 106.65 kB 0%
static/js/pl.js 88.64 kB 0%
static/js/pt-BR.js 154.57 kB 0%
static/js/sv.js 78.2 kB 0%
static/js/th.js 182.35 kB 0%
static/js/uk.js 215.11 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/BackgroundImage.js 120.54 kB 0%
static/js/ReportRouter.js 1.13 MB 0%
static/js/TransactionList.js 106.13 kB 0%
static/js/AppliedFilters.js 9.71 kB 0%
static/js/usePayeeRuleCounts.js 10.05 kB 0%
static/js/useTransactionBatchActions.js 13.23 kB 0%
static/js/FormulaEditor.js 1.04 MB 0%

loot-core

Total

Files count Total bundle size % Changed
1 5.86 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.hYlRL2CV.js 5.86 MB 0%

api

Total

Files count Total bundle size % Changed
1 4.4 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
bundle.api.js 4.4 MB 0%

- Fix notification banner to show accurate account counts during sync
  - For individual accounts: displays total queued requests (1, 2, 3...)
  - For 'Sync All': displays total accounts being synced

Fixes sync queue notification counts that were always showing '1 account remaining'
and improves overall bank sync UX consistency.
@actual-github-bot actual-github-bot bot changed the title fix: improve bank sync UX with accurate queue counts [WIP] fix: improve bank sync UX with accurate queue counts Dec 22, 2025
@netlify
Copy link

netlify bot commented Dec 22, 2025

Deploy Preview for actualbudget-website ready!

Name Link
🔨 Latest commit cbba32d
🔍 Latest deploy log https://app.netlify.com/projects/actualbudget-website/deploys/698e95a4917a8600080f158d
😎 Deploy Preview https://deploy-preview-6474.www.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Dec 22, 2025

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 8f34dde
🔍 Latest deploy log https://app.netlify.com/projects/actualbudget/deploys/6990af3b4dba440008699063
😎 Deploy Preview https://deploy-preview-6474.demo.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

Bundle Stats

desktop-client

Total

Files count Total bundle size % Changed
28 14.33 MB → 14.34 MB (+8.14 kB) +0.06%
Changeset
File Δ Size
src/hooks/useSyncAll.ts 🆕 +1.08 kB 0 B → 1.08 kB
src/components/banksync/index.tsx 📈 +2.03 kB (+40.03%) 5.06 kB → 7.09 kB
src/components/mobile/banksync/MobileBankSyncPage.tsx 📈 +2.15 kB (+36.14%) 5.95 kB → 8.1 kB
src/accounts/accountsSlice.ts 📈 +2.71 kB (+24.69%) 10.96 kB → 13.66 kB
src/components/BankSyncStatus.tsx 📈 +161 B (+6.74%) 2.33 kB → 2.49 kB
src/hooks/useAccountPreviewTransactions.ts 📈 +4 B (+0.06%) 6.5 kB → 6.5 kB
src/components/mobile/accounts/AccountsPage.tsx 📈 +8 B (+0.04%) 21.11 kB → 21.12 kB
src/components/transactions/SelectedTransactionsButton.tsx 📈 +4 B (+0.02%) 18.65 kB → 18.66 kB
src/components/accounts/Account.tsx 📈 +4 B (+0.01%) 48.56 kB → 48.57 kB
View detailed bundle breakdown

Added

Asset File Size % Changed
static/js/useSyncAll.js 0 B → 12.88 kB (+12.88 kB) -

Removed

Asset File Size % Changed
static/js/usePayeeRuleCounts.js 11.79 kB → 0 B (-11.79 kB) -100%

Bigger

Asset File Size % Changed
static/js/index.js 9.11 MB → 9.11 MB (+2.86 kB) +0.03%
static/js/narrow.js 635.89 kB → 638.05 kB (+2.16 kB) +0.34%
static/js/wide.js 158.95 kB → 160.99 kB (+2.03 kB) +1.28%

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
static/js/indexeddb-main-thread-worker-e59fee74.js 12.94 kB 0%
static/js/workbox-window.prod.es5.js 5.64 kB 0%
static/js/da.js 107.03 kB 0%
static/js/de.js 173.52 kB 0%
static/js/en-GB.js 7.24 kB 0%
static/js/en.js 158.4 kB 0%
static/js/es.js 174.34 kB 0%
static/js/fr.js 177.63 kB 0%
static/js/it.js 174.68 kB 0%
static/js/nb-NO.js 160.03 kB 0%
static/js/nl.js 103.57 kB 0%
static/js/pl.js 88.78 kB 0%
static/js/pt-BR.js 147.86 kB 0%
static/js/ru.js 108.05 kB 0%
static/js/sv.js 79.01 kB 0%
static/js/th.js 182.7 kB 0%
static/js/uk.js 219.26 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/BackgroundImage.js 120.48 kB 0%
static/js/ReportRouter.js 1.1 MB 0%
static/js/TransactionList.js 101.33 kB 0%
static/js/AppliedFilters.js 9.62 kB 0%
static/js/useTransactionBatchActions.js 13.23 kB 0%
static/js/FormulaEditor.js 1.04 MB 0%

loot-core

Total

Files count Total bundle size % Changed
1 5.79 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.DkeVR93i.js 5.79 MB 0%

api

Total

Files count Total bundle size % Changed
1 4.34 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
bundle.api.js 4.34 MB 0%

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

📝 Walkthrough

Walkthrough

Adds a queue-based account synchronization system with syncQueue and isProcessingQueue in Redux, new reducers and thunks to enqueue and sequentially process sync requests, and UI updates (desktop & mobile) adding a "Sync All" action and queue-aware status display.

Changes

Cohort / File(s) Summary
Account Synchronization Queue System
packages/desktop-client/src/accounts/accountsSlice.ts
Adds SyncRequest type, syncQueue and isProcessingQueue state; new reducers addToSyncQueue, removeFromSyncQueue, setProcessingQueue, clearSyncQueue; changes syncAccounts to enqueue requests; adds thunks processQueue and processSingleSync to atomically claim and process the queue sequentially (supports SimpleFin batch path, special-ID handling, per-account sync); logs per-item errors and ensures the processing flag is reset.
Synchronization Status UI
packages/desktop-client/src/components/BankSyncStatus.tsx
Reads accountsSyncing, syncQueue, isProcessingQueue from state.account; computes hasAllAccountsRequest and totalRemaining; derives showStatus from isProcessingQueue and remaining count; updates displayed remaining count and trigger logic.
Desktop Bank Sync UI
packages/desktop-client/src/components/banksync/index.tsx
Integrates useSyncAll, renders a "Sync All" Button when linked accounts exist; button label and disabled state bound to isProcessingQueue; shows processing status derived from syncQueue.
Mobile Bank Sync UI
packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
Adds mobile "Sync All" block using useSyncAll, with dynamic label/status and disabled state while isProcessingQueue is true; shows processing text reflecting ALL_ACCOUNTS vs per-queue counts.
Hook: Sync All
packages/desktop-client/src/hooks/useSyncAll.ts
New hook returning { linkedAccounts, handleSyncAll, isProcessingQueue }; computes linked accounts (non-closed with account_sync_source) and dispatches syncAccounts({}) (ALL_ACCOUNTS request) via handleSyncAll.
Release Notes
upcoming-release-notes/6474.md
Adds release note describing UI/UX changes for bank sync queue counts and "Sync All" consistency.

Sequence Diagram

sequenceDiagram
    participant UI as Desktop/Mobile UI
    participant Redux as accountsSlice (state)
    participant Processor as processQueue (thunk)
    participant Worker as processSingleSync (thunk)
    participant SyncSvc as Sync Service/API

    UI->>Redux: dispatch syncAccounts({}) or syncAccounts({ids})
    Redux->>Redux: addToSyncQueue(enqueue request(s))
    Redux->>Processor: trigger processQueue (if not processing)
    activate Processor
    Processor->>Redux: setProcessingQueue(true)
    loop per queued request
        Processor->>Worker: dispatch processSingleSync(request)
        activate Worker
        alt SimpleFin batch
            Worker->>SyncSvc: simplefin-batch-sync
            SyncSvc-->>Worker: batch results
        else Special-ID handling
            Worker->>SyncSvc: sync special account(s)
            SyncSvc-->>Worker: results
        else Per-account flow
            Worker->>SyncSvc: sync single account
            SyncSvc-->>Worker: results
        end
        Worker->>Redux: update newTransactions/matchedTransactions/updatedAccounts
        Worker->>Redux: removeFromSyncQueue(request.id)
        Worker-->>Processor: success or failure
        deactivate Worker
    end
    Processor->>Redux: clearSyncQueue()
    Processor->>Redux: setProcessingQueue(false)
    deactivate Processor
    Redux->>UI: updated state (accountsSyncing, syncQueue, isProcessingQueue)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I queue my hops, one at a time,
Through ledgers neat and sync sublime.
I nibble bugs and keep things bright,
One ordered hop to set things right.
Happy syncing, hop by hop!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately summarizes the main change: improving bank sync UX through accurate queue counts, which is the core objective of the changeset.
Description check ✅ Passed The PR description clearly relates to the changeset, describing the bank sync queue count fixes and UI improvements reflected in the modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (8)
packages/desktop-client/src/components/banksync/index.tsx (3)

14-23: Import order does not follow coding guidelines.

Per the coding guidelines, the import order should be: React → external packages → loot-core → parent/sibling/index imports → @desktop-client imports. The relative imports (AccountsHeader, AccountsList) should come before the @desktop-client imports.

Suggested import reorder
 import { AccountsHeader } from './AccountsHeader';
 import { AccountsList } from './AccountsList';
 
-import { syncAccounts } from '@desktop-client/accounts/accountsSlice';
-import { MOBILE_NAV_HEIGHT } from '@desktop-client/components/mobile/MobileNavTabs';
-import { Page } from '@desktop-client/components/Page';
-import { useAccounts } from '@desktop-client/hooks/useAccounts';
-import { useGlobalPref } from '@desktop-client/hooks/useGlobalPref';
-import { pushModal } from '@desktop-client/modals/modalsSlice';
-import { useDispatch, useSelector } from '@desktop-client/redux';
+
+import { syncAccounts } from '@desktop-client/accounts/accountsSlice';
+import { MOBILE_NAV_HEIGHT } from '@desktop-client/components/mobile/MobileNavTabs';
+import { Page } from '@desktop-client/components/Page';
+import { useAccounts } from '@desktop-client/hooks/useAccounts';
+import { useGlobalPref } from '@desktop-client/hooks/useGlobalPref';
+import { pushModal } from '@desktop-client/modals/modalsSlice';
+import { useDispatch, useSelector } from '@desktop-client/redux';

156-171: Use theme variables instead of hardcoded CSS custom properties.

Per coding guidelines: "Never import colors directly - use theme instead." The hardcoded var(--n1) and var(--n6) should be replaced with theme tokens (e.g., theme.pageText, theme.pageTextSubdued) for consistency with the mobile implementation and theming support.

Proposed fix using theme

First, import theme at the top:

+import { theme } from '@actual-app/components/theme';

Then update the Button styles:

 <Button
   variant="bare"
   onPress={handleSyncAll}
   isDisabled={isProcessingQueue}
   style={{
-    color: isProcessingQueue ? 'var(--n6)' : 'var(--n1)',
+    color: isProcessingQueue ? theme.pageTextSubdued : theme.pageText,
     textDecoration: 'underline',
     textDecorationColor: isProcessingQueue
-      ? 'var(--n6)'
-      : 'var(--n1)',
+      ? theme.pageTextSubdued
+      : theme.pageText,
     padding: 0,
     minHeight: 'auto',
   }}
 >

And the Text color:

-<Text style={{ color: 'var(--n6)' }}>
+<Text style={{ color: theme.pageTextSubdued }}>

172-182: Redundant condition check.

The linkedAccounts.length > 0 condition on line 172 is redundant since the parent View is already conditionally rendered only when linkedAccounts.length > 0 (line 147).

Proposed simplification
-            {linkedAccounts.length > 0 && (
-              <Text style={{ color: 'var(--n6)' }}>
+            <Text style={{ color: theme.pageTextSubdued }}>
                 {isProcessingQueue
                   ? t('Processing {{count}} linked accounts', {
                       count: linkedAccounts.length,
                     })
                   : t('{{count}} linked accounts', {
                       count: linkedAccounts.length,
                     })}
-              </Text>
-            )}
+            </Text>
packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx (2)

15-23: Import order does not follow coding guidelines.

Same issue as the desktop counterpart: relative imports should come before @desktop-client imports.

Suggested import reorder
 import { BankSyncAccountsList } from './BankSyncAccountsList';
 
-import { syncAccounts } from '@desktop-client/accounts/accountsSlice';
-import { Search } from '@desktop-client/components/common/Search';
+
+import { syncAccounts } from '@desktop-client/accounts/accountsSlice';
+import { Search } from '@desktop-client/components/common/Search';

56-65: Consider extracting shared Sync All logic.

The linkedAccounts memo and handleSyncAll callback are nearly identical between desktop (banksync/index.tsx) and mobile (MobileBankSyncPage.tsx). Consider extracting this into a shared custom hook (e.g., useSyncAllAccounts) to reduce duplication and ensure consistent behavior.

packages/desktop-client/src/accounts/accountsSlice.ts (3)

482-488: Replace console. with logger.*

Per coding guidelines: "Never use 'console.*' - use logger instead (enforced by ESLint rule 'actual/prefer-logger-over-console')."

Proposed fix

Import the logger and replace console calls:

+import { logger } from 'loot-core/platform/client/logger';

-        } catch (error) {
-          console.error(`Sync failed for ${request.id}:`, error);
+        } catch (error) {
+          logger.error(`Sync failed for ${request.id}:`, error);
-    } catch (error) {
-      console.error('Queue processing error:', error);
+    } catch (error) {
+      logger.error('Queue processing error:', error);

554-556: Replace console.log with logger.

Per coding guidelines, use logger instead of console.*.

Proposed fix
-      console.log('Using SimpleFin batch sync');
+      logger.info('Using SimpleFin batch sync');

628-630: Replace console.error with logger.

Proposed fix
-    } catch (error) {
-      console.error('Sync error:', error);
+    } catch (error) {
+      logger.error('Sync error:', error);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfea779 and f90df2d.

📒 Files selected for processing (4)
  • packages/desktop-client/src/accounts/accountsSlice.ts
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use TypeScript for all code
Use descriptive variable names with auxiliary verbs (e.g., 'isLoaded', 'hasError')
Use named exports for components and utilities; avoid default exports except in specific cases
Use functional and declarative programming patterns - avoid classes
Use the 'function' keyword for pure functions
Structure files with: exported component/page, helpers, static content, types
Organize imports in the following order: React imports, built-in Node.js modules, external packages, Actual packages, parent imports, sibling imports, index imports, with newlines between groups
Never use 'console.*' - use logger instead (enforced by ESLint rule 'actual/prefer-logger-over-console')
Never import from 'uuid' without destructuring - use 'import { v4 as uuidv4 } from 'uuid''

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Prefer 'type' over 'interface' for defining types in TypeScript
Avoid 'enum' - use objects or maps instead
Avoid 'any' or 'unknown' unless absolutely necessary
Avoid type assertions ('as', '!') - prefer 'satisfies' for type narrowing
Use inline type imports: 'import { type MyType } from '...''
Look for existing type definitions in 'packages/loot-core/src/types/' before creating new types

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{tsx,jsx}: Create new components in their own files
Don't use 'React.FunctionComponent' or 'React.FC' - type props directly
Don't use 'React.*' patterns - use named imports instead
Use '' instead of '' tags in React components
Avoid unstable nested components
Use 'satisfies' for type narrowing in React components
Use declarative JSX that is minimal and readable; avoid unnecessary curly braces in conditionals
Prefer explicit expressions ('condition && ') in JSX
Never import colors directly - use theme instead
Use 'Trans' component instead of 't()' function when possible for internationalization
All user-facing strings must be translated using i18n

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
packages/desktop-client/src/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

packages/desktop-client/src/**/*.{tsx,jsx}: Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')
Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🧠 Learnings (28)
📓 Common learnings
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-01-29T19:44:02.950Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4253
File: packages/desktop-client/src/components/banksync/EditSyncAccount.tsx:154-160
Timestamp: 2025-01-29T19:44:02.950Z
Learning: In EditSyncAccount.tsx, mappings state is initialized with defaultMappings (containing both 'payment' and 'deposit' directions) if no saved mappings exist, so direction maps are guaranteed to exist.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2024-10-04T05:13:58.322Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The `onReorder` function in `Accounts.tsx` was moved from `Sidebar.tsx`, and the `targetId` parameter remains typed as `unknown` intentionally.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
📚 Learning: 2024-11-04T00:34:13.035Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In `packages/loot-core/src/client/actions/account.ts`, within the `syncAccounts` function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2024-10-13T11:17:59.711Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3381
File: packages/desktop-client/src/components/budget/SidebarGroup.tsx:69-76
Timestamp: 2024-10-13T11:17:59.711Z
Learning: In the `SidebarGroup` component (`packages/desktop-client/src/components/budget/SidebarGroup.tsx`), context menu state management is handled in another file, ensuring that only one context menu is open at a time.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-10T02:29:05.655Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/Sidebar.tsx:112-116
Timestamp: 2024-10-10T02:29:05.655Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, the `BudgetName` component consists of three parts: `BudgetName`, `EditBudgetName`, and the Menu. Keeping `EditBudgetName` as a separate component helps maintain cleaner code by separating concerns.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: qedi-r
Repo: actualbudget/actual PR: 3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is best avoided, and we should almost always prefer using the useTranslation hook if possible.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to **/*.{tsx,jsx} : Use 'Trans' component instead of 't()' function when possible for internationalization

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-02-11T18:44:28.613Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 0
File: :0-0
Timestamp: 2025-02-11T18:44:28.613Z
Learning: The Button2 component is deprecated. Use actual-app/components/button instead.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-01-17T12:11:23.669Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3840
File: packages/loot-core/src/server/budget/template-notes.ts:12-12
Timestamp: 2025-01-17T12:11:23.669Z
Learning: In server-side code or non-React contexts where React hooks cannot be used, prefer using `import { t } from 'i18next'` and directly using `t(string)` for translations, instead of using react-i18next hooks or other approaches.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-10-22T16:22:17.482Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5978
File: packages/desktop-client/src/components/mobile/banksync/BankSyncAccountsListItem.tsx:58-67
Timestamp: 2025-10-22T16:22:17.482Z
Learning: In react-i18next, the Trans component supports interpolation using the syntax `<Trans>Text {{ variable: value }}</Trans>` where the double curly braces are special syntax recognized by react-i18next for variable substitution. This is valid and should not be flagged as incorrect.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-22T05:34:56.976Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-10-22T05:34:56.976Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to **/*.{tsx,jsx} : All user-facing strings must be translated using i18n

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-06-21T04:04:01.231Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5206
File: packages/desktop-client/src/components/Titlebar.tsx:63-63
Timestamp: 2025-06-21T04:04:01.231Z
Learning: In react-i18next Trans components, double curly braces `{{ }}` are the correct syntax for interpolation placeholders, not JSX object literals. For example: `<Trans count={value}>{{ value }} items</Trans>` is the proper way to interpolate values within translation strings.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2024-10-22T05:32:30.530Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-10-22T05:32:30.530Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-16T21:27:47.818Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6428
File: eslint.config.mjs:249-278
Timestamp: 2025-12-16T21:27:47.818Z
Learning: Enforce the specified import order for all TypeScript/JavaScript files under packages/desktop-client: React imports, then built-in Node.js modules, then external packages, then loot-core packages, then parent imports, then sibling imports, then index imports, and finally desktop-client imports (desktop-client imports come after relative imports, not before). Apply this rule to all relevant files in that directory (extensions: .ts, .tsx, .js, .mjs).

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/BankSyncStatus.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2024-11-04T14:14:10.698Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/types/models/bank-sync.d.ts:11-21
Timestamp: 2024-11-04T14:14:10.698Z
Learning: In `packages/loot-core/src/types/models/bank-sync.d.ts`, when defining TypeScript types for data received from the server, maintain the field names as they are in the server response, even if they don't follow TypeScript naming conventions.

Applied to files:

  • packages/desktop-client/src/components/BankSyncStatus.tsx
📚 Learning: 2025-10-24T21:29:26.090Z
Learnt from: csenel
Repo: actualbudget/actual PR: 5991
File: packages/desktop-client/src/components/mobile/budget/BudgetCell.tsx:82-89
Timestamp: 2025-10-24T21:29:26.090Z
Learning: Notification messages in BudgetCell.tsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.

Applied to files:

  • packages/desktop-client/src/components/BankSyncStatus.tsx
📚 Learning: 2025-09-30T19:23:01.635Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5821
File: packages/desktop-client/src/components/accounts/BalanceHistoryGraph.tsx:76-91
Timestamp: 2025-09-30T19:23:01.635Z
Learning: In the Actual Budget codebase, `query.transactions(undefined)` correctly handles the "all accounts" view and is the proper way to query all transactions, as opposed to using `q('transactions')` with filters directly.

Applied to files:

  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2024-10-21T02:24:38.823Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3699
File: packages/loot-core/src/client/actions/app.ts:56-74
Timestamp: 2024-10-21T02:24:38.823Z
Learning: The team has decided to handle the `/accounts` route in the default case within the `getPageDocs` function in `packages/loot-core/src/client/actions/app.ts`, as discussed in a previous PR.

Applied to files:

  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.

Applied to files:

  • packages/desktop-client/src/accounts/accountsSlice.ts
📚 Learning: 2025-12-19T20:22:15.654Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6450
File: packages/crdt/src/crdt/merkle.ts:10-10
Timestamp: 2025-12-19T20:22:15.654Z
Learning: In TypeScript reviews, differentiate between type-only imports and runtime identifiers. If an import is only used in type annotations, type positions (parameters, return types, type aliases) should verify it's truly type-only and not causing runtime imports. Do not conflate a type name (e.g., Timestamp) with a nearby lowercased variable (e.g., timestamp) that is used at runtime. This distinction should be checked across .ts files (e.g., packages/**/src/**/*.ts) to ensure type-only imports do not introduce runtime dependencies or ambiguity.

Applied to files:

  • packages/desktop-client/src/accounts/accountsSlice.ts
🧬 Code graph analysis (3)
packages/desktop-client/src/components/banksync/index.tsx (2)
packages/desktop-client/src/redux/index.ts (1)
  • useSelector (19-19)
packages/desktop-client/src/accounts/accountsSlice.ts (1)
  • syncAccounts (413-438)
packages/desktop-client/src/components/BankSyncStatus.tsx (1)
packages/desktop-client/src/redux/index.ts (1)
  • useSelector (19-19)
packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx (4)
packages/desktop-client/src/redux/index.ts (1)
  • useSelector (19-19)
packages/desktop-client/src/accounts/accountsSlice.ts (1)
  • syncAccounts (413-438)
packages/component-library/src/View.tsx (1)
  • View (14-33)
packages/component-library/src/Text.tsx (1)
  • Text (19-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: Redirect rules - actualbudget-website
  • GitHub Check: Header rules - actualbudget-website
  • GitHub Check: Pages changed - actualbudget-website
  • GitHub Check: api
  • GitHub Check: web
  • GitHub Check: build (windows-latest)
  • GitHub Check: build (macos-latest)
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: Analyze
  • GitHub Check: compare
  • GitHub Check: Functional (shard 1/5)
  • GitHub Check: test
  • GitHub Check: validate-cli
  • GitHub Check: Visual regression (shard 2/5)
  • GitHub Check: Functional (shard 4/5)
  • GitHub Check: Functional (shard 3/5)
  • GitHub Check: Visual regression (shard 5/5)
  • GitHub Check: Functional Desktop App
  • GitHub Check: Visual regression (shard 4/5)
  • GitHub Check: Functional (shard 2/5)
  • GitHub Check: Functional (shard 5/5)
  • GitHub Check: Visual regression (shard 3/5)
  • GitHub Check: Visual regression (shard 1/5)
🔇 Additional comments (5)
packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx (1)

170-213: LGTM!

The mobile Sync All button implementation correctly uses theme tokens and follows i18n patterns with the t() function for translations.

packages/desktop-client/src/components/BankSyncStatus.tsx (1)

18-26: LGTM on the queue count logic.

The logic correctly distinguishes between "Sync All" (uses accountsSyncing.length for actual accounts being processed) and individual account syncs (uses syncQueue.length for queued requests). This addresses the PR objective of preventing double-counting.

packages/desktop-client/src/accounts/accountsSlice.ts (3)

440-496: Queue processing logic looks solid.

The implementation correctly:

  1. Uses atomic check-and-set via Redux state to prevent concurrent processing
  2. Processes items sequentially in FIFO order
  3. Removes items from queue after processing (success or failure)
  4. Always resets isProcessingQueue in the finally block

One minor observation: if sync requests are added faster than they're processed, the queue will grow, but this is acceptable given the sequential processing requirement for bank syncs.


116-137: LGTM on queue management reducers.

The reducers correctly handle:

  • Duplicate prevention in addToSyncQueue
  • Proper filtering in removeFromSyncQueue
  • Clean state reset in clearSyncQueue

561-591: LGTM on progressive SimpleFin batch processing.

The implementation correctly updates accountsSyncing progressively as each SimpleFin account completes, which addresses the PR objective of avoiding abrupt count disappearance during "Sync All" operations.

@github-actions
Copy link
Contributor

🤖 Auto-generated Release Notes

Hey @robertlestak! I've automatically created a release notes file based on CodeRabbit's analysis:

Category: Enhancements
Summary: Improve bank synchronization UX by implementing a queue system for accurate processing and status updates.
File: upcoming-release-notes/6474.md

If you're happy with this release note, you can add it to your pull request. If not, you'll need to add your own before a maintainer can review your change.

- Fix import order to follow project guidelines (React → external → loot-core → relative → @desktop-client)
- Replace hardcoded CSS variables with theme tokens for consistency
- Remove redundant condition check in desktop sync status display
- Keep console.* calls as ESLint rule only applies to server-side code
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f90df2d and 130ff64.

📒 Files selected for processing (2)
  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use TypeScript for all code
Use descriptive variable names with auxiliary verbs (e.g., 'isLoaded', 'hasError')
Use named exports for components and utilities; avoid default exports except in specific cases
Use functional and declarative programming patterns - avoid classes
Use the 'function' keyword for pure functions
Structure files with: exported component/page, helpers, static content, types
Organize imports in the following order: React imports, built-in Node.js modules, external packages, Actual packages, parent imports, sibling imports, index imports, with newlines between groups
Never use 'console.*' - use logger instead (enforced by ESLint rule 'actual/prefer-logger-over-console')
Never import from 'uuid' without destructuring - use 'import { v4 as uuidv4 } from 'uuid''

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Prefer 'type' over 'interface' for defining types in TypeScript
Avoid 'enum' - use objects or maps instead
Avoid 'any' or 'unknown' unless absolutely necessary
Avoid type assertions ('as', '!') - prefer 'satisfies' for type narrowing
Use inline type imports: 'import { type MyType } from '...''
Look for existing type definitions in 'packages/loot-core/src/types/' before creating new types

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{tsx,jsx}: Create new components in their own files
Don't use 'React.FunctionComponent' or 'React.FC' - type props directly
Don't use 'React.*' patterns - use named imports instead
Use '' instead of '' tags in React components
Avoid unstable nested components
Use 'satisfies' for type narrowing in React components
Use declarative JSX that is minimal and readable; avoid unnecessary curly braces in conditionals
Prefer explicit expressions ('condition && ') in JSX
Never import colors directly - use theme instead
Use 'Trans' component instead of 't()' function when possible for internationalization
All user-facing strings must be translated using i18n

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
packages/desktop-client/src/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

packages/desktop-client/src/**/*.{tsx,jsx}: Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')
Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
🧠 Learnings (19)
📓 Common learnings
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: csenel
Repo: actualbudget/actual PR: 5991
File: packages/desktop-client/src/components/mobile/budget/BudgetCell.tsx:82-89
Timestamp: 2025-10-24T21:29:26.090Z
Learning: Notification messages in BudgetCell.tsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Learnt from: misu-dev
Repo: actualbudget/actual PR: 5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:45:40.807Z
Learning: The user misu-dev prefers strict type checking for financial format types in useFormat.ts as a long-term goal, but acknowledges that creating follow-up issues for cleanup should wait until after the current PR is merged, not during the development phase.
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In `packages/loot-core/src/client/actions/account.ts`, within the `syncAccounts` function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-11-04T00:34:13.035Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In `packages/loot-core/src/client/actions/account.ts`, within the `syncAccounts` function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-01-29T19:44:02.950Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4253
File: packages/desktop-client/src/components/banksync/EditSyncAccount.tsx:154-160
Timestamp: 2025-01-29T19:44:02.950Z
Learning: In EditSyncAccount.tsx, mappings state is initialized with defaultMappings (containing both 'payment' and 'deposit' directions) if no saved mappings exist, so direction maps are guaranteed to exist.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: qedi-r
Repo: actualbudget/actual PR: 3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is best avoided, and we should almost always prefer using the useTranslation hook if possible.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to **/*.{tsx,jsx} : Use 'Trans' component instead of 't()' function when possible for internationalization

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-01-17T12:11:23.669Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3840
File: packages/loot-core/src/server/budget/template-notes.ts:12-12
Timestamp: 2025-01-17T12:11:23.669Z
Learning: In server-side code or non-React contexts where React hooks cannot be used, prefer using `import { t } from 'i18next'` and directly using `t(string)` for translations, instead of using react-i18next hooks or other approaches.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-02-11T18:44:28.613Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 0
File: :0-0
Timestamp: 2025-02-11T18:44:28.613Z
Learning: The Button2 component is deprecated. Use actual-app/components/button instead.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to **/*.{tsx,jsx} : All user-facing strings must be translated using i18n

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-10-22T16:22:17.482Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5978
File: packages/desktop-client/src/components/mobile/banksync/BankSyncAccountsListItem.tsx:58-67
Timestamp: 2025-10-22T16:22:17.482Z
Learning: In react-i18next, the Trans component supports interpolation using the syntax `<Trans>Text {{ variable: value }}</Trans>` where the double curly braces are special syntax recognized by react-i18next for variable substitution. This is valid and should not be flagged as incorrect.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-22T05:32:30.530Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-10-22T05:32:30.530Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-22T05:34:56.976Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-10-22T05:34:56.976Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-04T05:13:58.322Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The `onReorder` function in `Accounts.tsx` was moved from `Sidebar.tsx`, and the `targetId` parameter remains typed as `unknown` intentionally.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-12-16T21:27:47.818Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6428
File: eslint.config.mjs:249-278
Timestamp: 2025-12-16T21:27:47.818Z
Learning: Enforce the specified import order for all TypeScript/JavaScript files under packages/desktop-client: React imports, then built-in Node.js modules, then external packages, then loot-core packages, then parent imports, then sibling imports, then index imports, and finally desktop-client imports (desktop-client imports come after relative imports, not before). Apply this rule to all relevant files in that directory (extensions: .ts, .tsx, .js, .mjs).

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: build (macos-latest)
  • GitHub Check: build (windows-latest)
  • GitHub Check: Visual regression (shard 5/5)
  • GitHub Check: Visual regression (shard 2/5)
  • GitHub Check: Visual regression (shard 4/5)
  • GitHub Check: Functional (shard 4/5)
  • GitHub Check: Functional (shard 3/5)
  • GitHub Check: Functional (shard 5/5)
  • GitHub Check: Functional (shard 2/5)
  • GitHub Check: Visual regression (shard 3/5)
  • GitHub Check: Functional Desktop App
  • GitHub Check: Functional (shard 1/5)
  • GitHub Check: Visual regression (shard 1/5)
  • GitHub Check: web
  • GitHub Check: test
  • GitHub Check: Analyze
  • GitHub Check: validate-cli
  • GitHub Check: compare

- Convert status text strings to use Trans component with interpolation
- Follow project guidelines to prefer Trans over t() function when possible
- Use correct react-i18next syntax: <Trans count={value}>Text {{ count: value }}</Trans>
@github-actions

This comment has been minimized.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/desktop-client/src/components/banksync/index.tsx (1)

51-67: LGTM!

State selection and the handleSyncAll callback are correctly implemented. The pattern mirrors the mobile implementation appropriately.

Consider extracting the shared linkedAccounts computation and handleSyncAll callback into a reusable hook (e.g., useSyncAll) since both this file and MobileBankSyncPage.tsx implement identical logic. This would improve maintainability and ensure consistent behavior across desktop and mobile.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 130ff64 and c123157.

📒 Files selected for processing (2)
  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use TypeScript for all code
Use descriptive variable names with auxiliary verbs (e.g., 'isLoaded', 'hasError')
Use named exports for components and utilities; avoid default exports except in specific cases
Use functional and declarative programming patterns - avoid classes
Use the 'function' keyword for pure functions
Structure files with: exported component/page, helpers, static content, types
Organize imports in the following order: React imports, built-in Node.js modules, external packages, Actual packages, parent imports, sibling imports, index imports, with newlines between groups
Never use 'console.*' - use logger instead (enforced by ESLint rule 'actual/prefer-logger-over-console')
Never import from 'uuid' without destructuring - use 'import { v4 as uuidv4 } from 'uuid''

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Prefer 'type' over 'interface' for defining types in TypeScript
Avoid 'enum' - use objects or maps instead
Avoid 'any' or 'unknown' unless absolutely necessary
Avoid type assertions ('as', '!') - prefer 'satisfies' for type narrowing
Use inline type imports: 'import { type MyType } from '...''
Look for existing type definitions in 'packages/loot-core/src/types/' before creating new types

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{tsx,jsx}: Create new components in their own files
Don't use 'React.FunctionComponent' or 'React.FC' - type props directly
Don't use 'React.*' patterns - use named imports instead
Use '' instead of '' tags in React components
Avoid unstable nested components
Use 'satisfies' for type narrowing in React components
Use declarative JSX that is minimal and readable; avoid unnecessary curly braces in conditionals
Prefer explicit expressions ('condition && ') in JSX
Never import colors directly - use theme instead
Use 'Trans' component instead of 't()' function when possible for internationalization
All user-facing strings must be translated using i18n

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
packages/desktop-client/src/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

packages/desktop-client/src/**/*.{tsx,jsx}: Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')
Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux

Files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🧠 Learnings (24)
📓 Common learnings
Learnt from: csenel
Repo: actualbudget/actual PR: 5991
File: packages/desktop-client/src/components/mobile/budget/BudgetCell.tsx:82-89
Timestamp: 2025-10-24T21:29:26.090Z
Learning: Notification messages in BudgetCell.tsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4181
File: packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx:252-254
Timestamp: 2025-01-18T20:08:55.203Z
Learning: Notification messages in BudgetTable.jsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Learnt from: misu-dev
Repo: actualbudget/actual PR: 5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:45:40.807Z
Learning: The user misu-dev prefers strict type checking for financial format types in useFormat.ts as a long-term goal, but acknowledges that creating follow-up issues for cleanup should wait until after the current PR is merged, not during the development phase.
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-11-04T00:34:13.035Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In `packages/loot-core/src/client/actions/account.ts`, within the `syncAccounts` function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to **/*.{tsx,jsx} : All user-facing strings must be translated using i18n

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to **/*.{tsx,jsx} : Use 'Trans' component instead of 't()' function when possible for internationalization

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-10-24T21:29:26.090Z
Learnt from: csenel
Repo: actualbudget/actual PR: 5991
File: packages/desktop-client/src/components/mobile/budget/BudgetCell.tsx:82-89
Timestamp: 2025-10-24T21:29:26.090Z
Learning: Notification messages in BudgetCell.tsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-01-18T20:08:55.203Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4181
File: packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx:252-254
Timestamp: 2025-01-18T20:08:55.203Z
Learning: Notification messages in BudgetTable.jsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-11-22T17:03:41.876Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6158
File: packages/desktop-client/src/hooks/useScheduleEdit.ts:163-172
Timestamp: 2025-11-22T17:03:41.876Z
Learning: In packages/desktop-client/src/hooks/useScheduleEdit.ts around lines 163-172, the 'set-transactions' case uses a single-argument sort comparator (`transactions.sort(a => { return action.transactionId === a.id ? -1 : 1; })`), which is not symmetric and can produce unstable ordering. The correct approach is to use a two-argument comparator that returns -1/0/1 based on whether a or b matches action.transactionId. MatissJanis prefers to fix this in a future PR when this code is modified again.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: qedi-r
Repo: actualbudget/actual PR: 3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is best avoided, and we should almost always prefer using the useTranslation hook if possible.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-01-17T12:11:23.669Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3840
File: packages/loot-core/src/server/budget/template-notes.ts:12-12
Timestamp: 2025-01-17T12:11:23.669Z
Learning: In server-side code or non-React contexts where React hooks cannot be used, prefer using `import { t } from 'i18next'` and directly using `t(string)` for translations, instead of using react-i18next hooks or other approaches.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-02-11T18:44:28.613Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 0
File: :0-0
Timestamp: 2025-02-11T18:44:28.613Z
Learning: The Button2 component is deprecated. Use actual-app/components/button instead.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-10-22T16:22:17.482Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5978
File: packages/desktop-client/src/components/mobile/banksync/BankSyncAccountsListItem.tsx:58-67
Timestamp: 2025-10-22T16:22:17.482Z
Learning: In react-i18next, the Trans component supports interpolation using the syntax `<Trans>Text {{ variable: value }}</Trans>` where the double curly braces are special syntax recognized by react-i18next for variable substitution. This is valid and should not be flagged as incorrect.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-04T19:58:16.062Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.062Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-22T05:32:30.530Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-10-22T05:32:30.530Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2024-10-22T05:34:56.976Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-10-22T05:34:56.976Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2024-10-04T05:13:58.322Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The `onReorder` function in `Accounts.tsx` was moved from `Sidebar.tsx`, and the `targetId` parameter remains typed as `unknown` intentionally.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
📚 Learning: 2025-01-29T19:44:02.950Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4253
File: packages/desktop-client/src/components/banksync/EditSyncAccount.tsx:154-160
Timestamp: 2025-01-29T19:44:02.950Z
Learning: In EditSyncAccount.tsx, mappings state is initialized with defaultMappings (containing both 'payment' and 'deposit' directions) if no saved mappings exist, so direction maps are guaranteed to exist.

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-12-16T21:27:47.818Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6428
File: eslint.config.mjs:249-278
Timestamp: 2025-12-16T21:27:47.818Z
Learning: Enforce the specified import order for all TypeScript/JavaScript files under packages/desktop-client: React imports, then built-in Node.js modules, then external packages, then loot-core packages, then parent imports, then sibling imports, then index imports, and finally desktop-client imports (desktop-client imports come after relative imports, not before). Apply this rule to all relevant files in that directory (extensions: .ts, .tsx, .js, .mjs).

Applied to files:

  • packages/desktop-client/src/components/banksync/index.tsx
  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-06-21T04:04:01.231Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5206
File: packages/desktop-client/src/components/Titlebar.tsx:63-63
Timestamp: 2025-06-21T04:04:01.231Z
Learning: In react-i18next Trans components, double curly braces `{{ }}` are the correct syntax for interpolation placeholders, not JSX object literals. For example: `<Trans count={value}>{{ value }} items</Trans>` is the proper way to interpolate values within translation strings.

Applied to files:

  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
📚 Learning: 2025-09-30T19:23:01.635Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5821
File: packages/desktop-client/src/components/accounts/BalanceHistoryGraph.tsx:76-91
Timestamp: 2025-09-30T19:23:01.635Z
Learning: In the Actual Budget codebase, `query.transactions(undefined)` correctly handles the "all accounts" view and is the proper way to query all transactions, as opposed to using `q('transactions')` with filters directly.

Applied to files:

  • packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx
🧬 Code graph analysis (2)
packages/desktop-client/src/components/banksync/index.tsx (3)
packages/desktop-client/src/redux/index.ts (1)
  • useSelector (19-19)
packages/loot-core/src/types/models/account.ts (1)
  • AccountEntity (1-9)
packages/desktop-client/src/accounts/accountsSlice.ts (1)
  • syncAccounts (413-438)
packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx (2)
packages/desktop-client/src/redux/index.ts (1)
  • useSelector (19-19)
packages/desktop-client/src/accounts/accountsSlice.ts (1)
  • syncAccounts (413-438)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: build (windows-latest)
  • GitHub Check: build (macos-latest)
  • GitHub Check: Analyze
  • GitHub Check: Functional (shard 4/5)
  • GitHub Check: Functional (shard 3/5)
  • GitHub Check: Functional (shard 5/5)
  • GitHub Check: web
  • GitHub Check: Visual regression (shard 1/5)
  • GitHub Check: Functional (shard 1/5)
  • GitHub Check: Visual regression (shard 5/5)
  • GitHub Check: Functional (shard 2/5)
  • GitHub Check: Visual regression (shard 4/5)
  • GitHub Check: Visual regression (shard 2/5)
  • GitHub Check: Visual regression (shard 3/5)
  • GitHub Check: Functional Desktop App
  • GitHub Check: validate-cli
  • GitHub Check: test
  • GitHub Check: compare
🔇 Additional comments (6)
packages/desktop-client/src/components/mobile/banksync/MobileBankSyncPage.tsx (4)

4-23: Import ordering follows guidelines correctly.

The imports are properly organized: React imports first (lines 1-2), then external packages from @actual-app (lines 4-8), then loot-core types (lines 10-13), then sibling imports (line 15), and finally @desktop-client imports (lines 17-23). This aligns with the coding guidelines.


48-49: LGTM!

Correctly uses the typed useSelector from @desktop-client/redux as per coding guidelines, and properly destructures the queue state.


56-65: LGTM!

The linkedAccounts memo correctly filters for syncable accounts, and handleSyncAll properly dispatches syncAccounts({}) to trigger the ALL_ACCOUNTS queue flow. Using linkedAccounts.length in the dependency array is an appropriate optimization since only the count matters for the conditional.


170-221: LGTM!

The Sync All UI block is well-implemented:

  • Conditional rendering based on linkedAccounts.length > 0
  • Button correctly disabled during queue processing
  • Trans component with count prop follows react-i18next pluralization patterns
  • Status text appropriately differentiates between ALL_ACCOUNTS sync and individual account syncs

The {{ count: linkedAccounts.length }} syntax is valid react-i18next interpolation. Based on learnings, this double curly brace syntax is recognized by react-i18next for variable substitution.

packages/desktop-client/src/components/banksync/index.tsx (2)

4-24: LGTM!

Imports are correctly organized following the coding guidelines: React imports, external packages, loot-core types, sibling imports, and finally @desktop-client imports. The use of useDispatch and useSelector from @desktop-client/redux is correct per guidelines.


146-194: LGTM - Pluralization approach updated correctly.

The status text now uses the Trans component with the count prop, which is the correct approach for pluralization in react-i18next. This addresses the previous review feedback about improper pluralization with t().

The <Trans count={linkedAccounts.length}>{{ count: linkedAccounts.length }} linked accounts</Trans> syntax properly enables i18n pluralization when the translation files define _one and _other variants.

robertlestak and others added 3 commits December 22, 2025 11:33
- Add release notes file for PR #6474
- Categorize as Enhancement with user-facing description
- Follow project release notes format and guidelines
- Create reusable useSyncAll hook to share logic between desktop and mobile
- Extract linkedAccounts computation and handleSyncAll callback
- Improve maintainability and ensure consistent behavior across platforms
- Address CodeRabbit nitpick comment about code duplication
@robertlestak robertlestak changed the title [WIP] fix: improve bank sync UX with accurate queue counts fix: improve bank sync UX with accurate queue counts Dec 22, 2025
robertlestak and others added 9 commits December 22, 2025 11:50
- Change 'Processing X account' to 'Processing X accounts' for proper i18n pluralization
- Trans component with count prop automatically handles singular/plural forms
- Ensures consistent text display regardless of sync queue size
- Replace hardcoded 'Syncing... X account(s) remaining' with Trans component
- Import Trans from react-i18next for proper i18n support
- Use count prop for automatic pluralization handling
- Removes manual pluralization logic in favor of i18n system
@robertlestak
Copy link
Author

A new bug now: when I sync multiple accounts - only a single one at a time appears as pending (i.e. yellow dot in side-nav). Instead all the pending accounts should show this icon. And then it turns into green once the sync is complete.

fixed in latest commit, thanks for reviewing!

@MatissJanis
Copy link
Member

Great! The last issue I reported was addressed. Thank you so much for being so patient with me. Unfortunately I found one more bug.

  1. sync "on budget" accounts
  2. wait for one of these accounts to finish sync and the next to start off
  3. go to "all accounts" and sync this one too
  4. all accounts start syncing (this is correct)
  5. after a while - all the on-budget accounts are complete
  6. the 1st on-budget account that previously had finished syncing still shows a yellow dot; the sync is complete

robertlestak and others added 2 commits February 12, 2026 15:18
Fixed a bug where accounts would show as pending (yellow dot) even after
sync completed. This occurred when triggering multiple sync operations
(e.g., sync on-budget accounts, then sync all accounts while first sync
is still running).

Root causes:
1. processQueue only processed an initial snapshot of the queue, leaving
   items added during processing orphaned
2. addToSyncQueue didn't check if accounts were currently syncing,
   allowing already-synced accounts to be re-added to the queue

Fixes:
1. Changed processQueue to use a while loop that continues until the
   queue is completely empty, ensuring all accounts are processed
2. Enhanced addToSyncQueue to check both the queue AND currently syncing
   accounts before adding, preventing duplicates

This ensures the pending status indicator (yellow dot) accurately
reflects the actual sync state of all accounts.
Comment on lines 416 to 420
// An account is pending if it's in the sync queue or currently syncing
const pendingAccountIds = new Set([
...syncingAccountIds,
...syncQueue.map(req => req.id),
]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 suggestion: ‏this seems to be duplicated in many places. Can we not rely on only syncQueue? And then remove an item from the queue after the sync for it is finished.
That should help us reduce the duplication.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet resolved

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you introduced a utility that is shared among all the places, but the suggestion was to exclusively rely on syncQueue to get the pending account ids. Thus removing the need to use new Set() and the utility that was introduced.

robertlestak and others added 4 commits February 13, 2026 10:18
This commit addresses all feedback from the latest code review:

1. SimpleFin batch sync: Implemented proper batching of SimpleFin accounts
   in the queue processor to avoid rate limits. SimpleFin accounts are now
   detected and batched together, calling simplefin-batch-sync API instead
   of syncing individually.

2. Safer loop pattern: Changed while(true) to while(queue.length > 0) for
   explicit exit condition, making the code less prone to infinite loops.

3. Removed unused code: Deleted the clearSyncQueue reducer that was never
   called or exported.

4. Reduced duplication: Created getPendingAccountIds selector to eliminate
   duplicated logic across Accounts.tsx, AccountsPage.tsx, and Account.tsx.

5. Fixed yellow dot bug: Restructured sync state management to set
   accountsSyncing before async operations and use try/finally blocks to
   ensure proper cleanup, preventing accounts from showing as pending after
   sync completes.
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 13, 2026
- Remove getPendingAccountIds utility function as requested by maintainer
- Update all UI components to use only syncQueue for pending status
- Convert to Set where .has() method is used
- Fix while loop condition to use currentQueue.length for safer iteration

Addresses maintainer feedback to exclusively rely on syncQueue for
determining pending account IDs, simplifying the codebase and eliminating
potential race conditions.
@MatissJanis
Copy link
Member

👋 did you mean to close the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants