Balance sheet cache layer, non-blocking sync UI (#2356)

* Balance sheet cache layer with cache-busting

* Update family cache timestamps during Sync

* Less blocking sync loaders

* Consolidate family data caching key logic

* Fix turbo stream broadcasts

* Remove dev delay

* Add back account group sorting

app/models/account.rb

@@ -61,18 +61,6 @@ class Account < ApplicationRecord
     end
   end
 
-  def syncing?
-    self_syncing = syncs.visible.any?
-
-    # Since Plaid Items sync as a "group", if the item is syncing, even if the account
-    # sync hasn't yet started (i.e. we're still fetching the Plaid data), show it as syncing in UI.
-    if linked?
-      plaid_account&.plaid_item&.syncing? || self_syncing
-    else
-      self_syncing
-    end
-  end
-
   def institution_domain
     url_string = plaid_account&.plaid_item&.institution_url
     return nil unless url_string.present?

app/models/account/chartable.rb

@@ -24,20 +24,10 @@ module Account::Chartable
   end
 
   def sparkline_series
-    cache_key = family.build_cache_key("#{id}_sparkline")
+    cache_key = family.build_cache_key("#{id}_sparkline", invalidate_on_data_updates: true)
 
-    Rails.cache.fetch(cache_key) do
+    Rails.cache.fetch(cache_key, expires_in: 24.hours) do
       balance_series
     end
-  rescue => e
-    Rails.logger.error "Sparkline series error for account #{id}: #{e.message}"
-    # Return empty series as fallback
-    Series.new(
-      start_date: 30.days.ago.to_date,
-      end_date: Date.current,
-      interval: "1 day",
-      values: [],
-      favorable_direction: favorable_direction
-    )
   end
 end

app/models/account/sync_complete_event.rb

@@ -16,13 +16,13 @@ class Account::SyncCompleteEvent
       locals: { account: account }
     )
 
-    # Replace the groups this account belongs to in the sidebar
-    account_group_ids.each do |id|
+    # Replace the groups this account belongs to in both desktop and mobile sidebars
+    sidebar_targets.each do |(tab, mobile_flag)|
       account.broadcast_replace_to(
         account.family,
-        target: id,
+        target: account_group.dom_id(tab: tab, mobile: mobile_flag),
         partial: "accounts/accountable_group",
-        locals: { account_group: account_group, open: true }
+        locals: { account_group: account_group, open: true, all_tab: tab == :all, mobile: mobile_flag }
       )
     end
 
@@ -37,18 +37,18 @@ class Account::SyncCompleteEvent
   end
 
   private
-    # The sidebar will show the account in both its classification tab and the "all" tab,
-    # so we need to broadcast to both.
-    def account_group_ids
-      unless account_group.present?
-        error = Error.new("Account #{account.id} is not part of an account group")
-        Rails.logger.warn(error.message)
-        Sentry.capture_exception(error, level: :warning)
-        return []
-      end
+    # Returns an array of [tab, mobile?] tuples that should receive an update.
+    # We broadcast to both the classification-specific tab and the "all" tab,
+    # for desktop (mobile: false) and mobile (mobile: true) variants.
+    def sidebar_targets
+      return [] unless account_group.present?
 
-      id = account_group.id
-      [ id, "#{account_group.classification}_#{id}" ]
+      [
+        [ account_group.classification.to_sym, false ],
+        [ :all, false ],
+        [ account_group.classification.to_sym, true ],
+        [ :all, true ]
+      ]
     end
 
     def account_group

app/models/assistant/function/get_balance_sheet.rb

@@ -31,11 +31,11 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function
         monthly_history: historical_data(period)
       },
       assets: {
-        current: family.balance_sheet.total_assets_money.format,
+        current: family.balance_sheet.assets.total_money.format,
         monthly_history: historical_data(period, classification: "asset")
       },
       liabilities: {
-        current: family.balance_sheet.total_liabilities_money.format,
+        current: family.balance_sheet.liabilities.total_money.format,
         monthly_history: historical_data(period, classification: "liability")
       },
       insights: insights_data
@@ -65,8 +65,8 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function
     end
 
     def insights_data
-      assets = family.balance_sheet.total_assets
-      liabilities = family.balance_sheet.total_liabilities
+      assets = family.balance_sheet.assets.total
+      liabilities = family.balance_sheet.liabilities.total
       ratio = liabilities.zero? ? 0 : (liabilities / assets.to_f)
 
       {

app/models/balance_sheet.rb

@@ -1,7 +1,7 @@
 class BalanceSheet
   include Monetizable
 
-  monetize :total_assets, :total_liabilities, :net_worth
+  monetize :net_worth
 
   attr_reader :family
 
@@ -9,99 +9,36 @@ class BalanceSheet
     @family = family
   end
 
-  def total_assets
-    totals_query.filter { |t| t.classification == "asset" }.sum(&:converted_balance)
+  def assets
+    @assets ||= ClassificationGroup.new(
+      classification: "asset",
+      currency: family.currency,
+      accounts: account_totals.asset_accounts
+    )
   end
 
-  def total_liabilities
-    totals_query.filter { |t| t.classification == "liability" }.sum(&:converted_balance)
-  end
-
-  def net_worth
-    total_assets - total_liabilities
+  def liabilities
+    @liabilities ||= ClassificationGroup.new(
+      classification: "liability",
+      currency: family.currency,
+      accounts: account_totals.liability_accounts
+    )
   end
 
   def classification_groups
-    Rails.cache.fetch(family.build_cache_key("bs_classification_groups")) do
-      asset_groups     = account_groups("asset")
-      liability_groups = account_groups("liability")
-
-      [
-        ClassificationGroup.new(
-          key: "asset",
-          display_name: "Assets",
-          icon: "plus",
-          total_money: total_assets_money,
-          account_groups: asset_groups,
-          syncing?: asset_groups.any?(&:syncing?)
-        ),
-        ClassificationGroup.new(
-          key: "liability",
-          display_name: "Debts",
-          icon: "minus",
-          total_money: total_liabilities_money,
-          account_groups: liability_groups,
-          syncing?: liability_groups.any?(&:syncing?)
-        )
-      ]
-    end
+    [ assets, liabilities ]
   end
 
-  def account_groups(classification = nil)
-    Rails.cache.fetch(family.build_cache_key("bs_account_groups_#{classification || 'all'}")) do
-      classification_accounts = classification ? totals_query.filter { |t| t.classification == classification } : totals_query
-      classification_total    = classification_accounts.sum(&:converted_balance)
+  def account_groups
+    [ assets.account_groups, liabilities.account_groups ].flatten
+  end
 
-      account_groups = classification_accounts.group_by(&:accountable_type)
-                                              .transform_keys { |k| Accountable.from_type(k) }
-
-      groups = account_groups.map do |accountable, accounts|
-        group_total = accounts.sum(&:converted_balance)
-
-        key = accountable.model_name.param_key
-
-        group = AccountGroup.new(
-          id: classification ? "#{classification}_#{key}_group" : "#{key}_group",
-          key: key,
-          name: accountable.display_name,
-          classification: accountable.classification,
-          total: group_total,
-          total_money: Money.new(group_total, currency),
-          weight: classification_total.zero? ? 0 : group_total / classification_total.to_d * 100,
-          missing_rates?: accounts.any? { |a| a.missing_rates? },
-          color: accountable.color,
-          syncing?: accounts.any?(&:is_syncing),
-          accounts: accounts.map do |account|
-            account
-          end.sort_by(&:converted_balance).reverse
-        )
-
-        group
-      end
-
-      groups.sort_by do |group|
-        manual_order = Accountable::TYPES
-        type_name    = group.key.camelize
-        manual_order.index(type_name) || Float::INFINITY
-      end
-    end
+  def net_worth
+    assets.total - liabilities.total
   end
 
   def net_worth_series(period: Period.last_30_days)
-    memo_key = [ period.start_date, period.end_date ].compact.join("_")
-
-    @net_worth_series ||= {}
-
-    account_ids = active_accounts.pluck(:id)
-
-    builder = (@net_worth_series[memo_key] ||= Balance::ChartSeriesBuilder.new(
-      account_ids: account_ids,
-      currency: currency,
-      period: period,
-      favorable_direction: "up"
-    ))
-
-    builder.balance_series
+    net_worth_series_builder.net_worth_series(period: period)
   end
 
   def currency
@@ -109,32 +46,19 @@ class BalanceSheet
   end
 
   def syncing?
-    classification_groups.any? { |group| group.syncing? }
+    sync_status_monitor.syncing?
   end
 
   private
-    ClassificationGroup = Struct.new(:key, :display_name, :icon, :total_money, :account_groups, :syncing?, keyword_init: true)
-    AccountGroup = Struct.new(:id, :key, :name, :accountable_type, :classification, :total, :total_money, :weight, :accounts, :color, :missing_rates?, :syncing?, keyword_init: true)
-
-    def active_accounts
-      family.accounts.active.with_attached_logo
+    def sync_status_monitor
+      @sync_status_monitor ||= SyncStatusMonitor.new(family)
     end
 
-    def totals_query
-      @totals_query ||= active_accounts
-            .joins(ActiveRecord::Base.sanitize_sql_array([ "LEFT JOIN exchange_rates ON exchange_rates.date = CURRENT_DATE AND accounts.currency = exchange_rates.from_currency AND exchange_rates.to_currency = ?", currency ]))
-            .joins(ActiveRecord::Base.sanitize_sql_array([
-              "LEFT JOIN syncs ON syncs.syncable_id = accounts.id AND syncs.syncable_type = 'Account' AND syncs.status IN (?) AND syncs.created_at > ?",
-              %w[pending syncing],
-              Sync::VISIBLE_FOR.ago
-            ]))
-            .select(
-              "accounts.*",
-              "SUM(accounts.balance * COALESCE(exchange_rates.rate, 1)) as converted_balance",
-              "COUNT(syncs.id) > 0 as is_syncing",
-              ActiveRecord::Base.sanitize_sql_array([ "COUNT(CASE WHEN accounts.currency <> ? AND exchange_rates.rate IS NULL THEN 1 END) as missing_rates", currency ])
-            )
-            .group(:classification, :accountable_type, :id)
-            .to_a
+    def account_totals
+      @account_totals ||= AccountTotals.new(family, sync_status_monitor: sync_status_monitor)
+    end
+
+    def net_worth_series_builder
+      @net_worth_series_builder ||= NetWorthSeriesBuilder.new(family)
     end
 end

app/models/balance_sheet/account_group.rb

@@ -0,0 +1,61 @@
+class BalanceSheet::AccountGroup
+  include Monetizable
+
+  monetize :total, as: :total_money
+
+  attr_reader :name, :color, :accountable_type, :accounts
+
+  def initialize(name:, color:, accountable_type:, accounts:, classification_group:)
+    @name = name
+    @color = color
+    @accountable_type = accountable_type
+    @accounts = accounts
+    @classification_group = classification_group
+  end
+
+  # A stable DOM id for this group.
+  # Example outputs:
+  #   dom_id(tab: :asset)               # => "asset_depository"
+  #   dom_id(tab: :all, mobile: true)   # => "mobile_all_depository"
+  #
+  # Keeping all of the logic here means the view layer and broadcaster only
+  # need to ask the object for its DOM id instead of rebuilding string
+  # fragments in multiple places.
+  def dom_id(tab: nil, mobile: false)
+    parts = []
+    parts << "mobile" if mobile
+    parts << (tab ? tab.to_s : classification.to_s)
+    parts << key
+    parts.compact.join("_")
+  end
+
+  def key
+    accountable_type.to_s.underscore
+  end
+
+  def total
+    accounts.sum(&:converted_balance)
+  end
+
+  def weight
+    return 0 if classification_group.total.zero?
+
+    total / classification_group.total.to_d * 100
+  end
+
+  def syncing?
+    accounts.any?(&:syncing?)
+  end
+
+  # "asset" or "liability"
+  def classification
+    classification_group.classification
+  end
+
+  def currency
+    classification_group.currency
+  end
+
+  private
+    attr_reader :classification_group
+end

app/models/balance_sheet/account_totals.rb

@@ -0,0 +1,63 @@
+class BalanceSheet::AccountTotals
+  def initialize(family, sync_status_monitor:)
+    @family = family
+    @sync_status_monitor = sync_status_monitor
+  end
+
+  def asset_accounts
+    @asset_accounts ||= account_rows.filter { |t| t.classification == "asset" }
+  end
+
+  def liability_accounts
+    @liability_accounts ||= account_rows.filter { |t| t.classification == "liability" }
+  end
+
+  private
+    attr_reader :family, :sync_status_monitor
+
+    AccountRow = Data.define(:account, :converted_balance, :is_syncing) do
+      def syncing? = is_syncing
+
+      # Allows Rails path helpers to generate URLs from the wrapper
+      def to_param = account.to_param
+      delegate_missing_to :account
+    end
+
+    def active_accounts
+      @active_accounts ||= family.accounts.active.with_attached_logo
+    end
+
+    def account_rows
+      @account_rows ||= query.map do |account_row|
+        AccountRow.new(
+          account: account_row,
+          converted_balance: account_row.converted_balance,
+          is_syncing: sync_status_monitor.account_syncing?(account_row)
+        )
+      end
+    end
+
+    def cache_key
+      family.build_cache_key(
+        "balance_sheet_account_rows",
+        invalidate_on_data_updates: true
+      )
+    end
+
+    def query
+      @query ||= Rails.cache.fetch(cache_key) do
+        active_accounts
+          .joins(ActiveRecord::Base.sanitize_sql_array([
+            "LEFT JOIN exchange_rates ON exchange_rates.date = ? AND accounts.currency = exchange_rates.from_currency AND exchange_rates.to_currency = ?",
+            Date.current,
+            family.currency
+          ]))
+          .select(
+            "accounts.*",
+            "SUM(accounts.balance * COALESCE(exchange_rates.rate, 1)) as converted_balance"
+          )
+          .group(:classification, :accountable_type, :id)
+          .to_a
+      end
+    end
+end

app/models/balance_sheet/classification_group.rb

@@ -0,0 +1,61 @@
+class BalanceSheet::ClassificationGroup
+  include Monetizable
+
+  monetize :total, as: :total_money
+
+  attr_reader :classification, :currency
+
+  def initialize(classification:, currency:, accounts:)
+    @classification = normalize_classification!(classification)
+    @name = name
+    @currency = currency
+    @accounts = accounts
+  end
+
+  def name
+    classification.titleize.pluralize
+  end
+
+  def icon
+    classification == "asset" ? "plus" : "minus"
+  end
+
+  def total
+    accounts.sum(&:converted_balance)
+  end
+
+  def syncing?
+    accounts.any?(&:syncing?)
+  end
+
+  # For now, we group by accountable type. This can be extended in the future to support arbitrary user groupings.
+  def account_groups
+    groups = accounts.group_by(&:accountable_type)
+                     .transform_keys { |at| Accountable.from_type(at) }
+                     .map do |accountable, account_rows|
+                       BalanceSheet::AccountGroup.new(
+                         name: accountable.display_name,
+                         color: accountable.color,
+                         accountable_type: accountable,
+                         accounts: account_rows,
+                         classification_group: self
+                       )
+                     end
+
+    # Sort the groups using the manual order defined by Accountable::TYPES so that
+    # the UI displays account groups in a predictable, domain-specific sequence.
+    groups.sort_by do |group|
+      manual_order = Accountable::TYPES
+      type_name    = group.key.camelize
+      manual_order.index(type_name) || Float::INFINITY
+    end
+  end
+
+  private
+    attr_reader :accounts
+
+    def normalize_classification!(classification)
+      raise ArgumentError, "Invalid classification: #{classification}" unless %w[asset liability].include?(classification)
+      classification
+    end
+end

app/models/balance_sheet/net_worth_series_builder.rb

@@ -0,0 +1,38 @@
+class BalanceSheet::NetWorthSeriesBuilder
+  def initialize(family)
+    @family = family
+  end
+
+  def net_worth_series(period: Period.last_30_days)
+    Rails.cache.fetch(cache_key(period)) do
+      builder = Balance::ChartSeriesBuilder.new(
+        account_ids: active_account_ids,
+        currency: family.currency,
+        period: period,
+        favorable_direction: "up"
+      )
+
+      builder.balance_series
+    end
+  end
+
+  private
+    attr_reader :family
+
+    def active_account_ids
+      @active_account_ids ||= family.accounts.active.with_attached_logo.pluck(:id)
+    end
+
+    def cache_key(period)
+      key = [
+        "balance_sheet_net_worth_series",
+        period.start_date,
+        period.end_date
+      ].compact.join("_")
+
+      family.build_cache_key(
+        key,
+        invalidate_on_data_updates: true
+      )
+    end
+end

app/models/balance_sheet/sync_status_monitor.rb

@@ -0,0 +1,35 @@
+class BalanceSheet::SyncStatusMonitor
+  def initialize(family)
+    @family = family
+  end
+
+  def syncing?
+    syncing_account_ids.any?
+  end
+
+  def account_syncing?(account)
+    syncing_account_ids.include?(account.id)
+  end
+
+  private
+    attr_reader :family
+
+    def syncing_account_ids
+      Rails.cache.fetch(cache_key) do
+        Sync.visible
+            .where(syncable_type: "Account", syncable_id: family.accounts.active.pluck(:id))
+            .pluck(:syncable_id)
+            .to_set
+      end
+    end
+
+    # We re-fetch the set of syncing IDs any time a sync that belongs to the family is started or completed.
+    # This ensures we're always fetching the latest sync statuses without re-querying on every page load in idle times (no syncs happening).
+    def cache_key
+      [
+        "balance_sheet_sync_status",
+        family.id,
+        family.latest_sync_activity_at
+      ].join("_")
+    end
+end

app/models/concerns/syncable.rb

@@ -6,7 +6,7 @@ module Syncable
   end
 
   def syncing?
-    raise NotImplementedError, "Subclasses must implement the syncing? method"
+    syncs.visible.any?
   end
 
   # Schedules a sync for syncable.  If there is an existing sync pending/syncing for this syncable,

app/models/family.rb

@@ -35,25 +35,6 @@ class Family < ApplicationRecord
   validates :locale, inclusion: { in: I18n.available_locales.map(&:to_s) }
   validates :date_format, inclusion: { in: DATE_FORMATS.map(&:last) }
 
-  # If any accounts or plaid items are syncing, the family is also syncing, even if a formal "Family Sync" is not running.
-  def syncing?
-    # Check for any in-progress syncs that belong directly to the family, to one of the
-    # family's accounts, or to one of the family's Plaid items. By moving the `visible`
-    # scope to the beginning we narrow down the candidate rows **before** performing the
-    # joins and by explicitly constraining the `syncable_type` for the direct Family
-    # match we allow Postgres to use the composite index on `(syncable_type, syncable_id)`.
-    Sync.visible
-        .joins("LEFT JOIN accounts ON accounts.id = syncs.syncable_id AND syncs.syncable_type = 'Account'")
-        .joins("LEFT JOIN plaid_items ON plaid_items.id = syncs.syncable_id AND syncs.syncable_type = 'PlaidItem'")
-        .where(
-          "(syncs.syncable_type = 'Family' AND syncs.syncable_id = :family_id) OR " \
-          "accounts.family_id = :family_id OR " \
-          "plaid_items.family_id = :family_id",
-          family_id: id
-        )
-        .exists?
-  end
-
   def assigned_merchants
     merchant_ids = transactions.where.not(merchant_id: nil).pluck(:merchant_id).uniq
     Merchant.where(id: merchant_ids)
@@ -110,13 +91,15 @@ class Family < ApplicationRecord
     entries.order(:date).first&.date || Date.current
   end
 
-  # Cache key that is invalidated when any of the family's entries are updated (which affect rollups and other calculations)
-  def build_cache_key(key)
+  def build_cache_key(key, invalidate_on_data_updates: false)
+    # Our data sync process updates this timestamp whenever any family account successfully completes a data update.
+    # By including it in the cache key, we can expire caches every time family account data changes.
+    data_invalidation_key = invalidate_on_data_updates ? latest_sync_completed_at : nil
+
     [
-      "family",
       id,
       key,
-      entries.maximum(:updated_at)
+      data_invalidation_key
     ].compact.join("_")
   end
 

app/models/plaid_item.rb

@@ -46,14 +46,6 @@ class PlaidItem < ApplicationRecord
     DestroyJob.perform_later(self)
   end
 
-  def syncing?
-    Sync.joins("LEFT JOIN accounts a ON a.id = syncs.syncable_id AND syncs.syncable_type = 'Account'")
-        .joins("LEFT JOIN plaid_accounts pa ON pa.id = a.plaid_account_id")
-        .where("syncs.syncable_id = ? OR pa.plaid_item_id = ?", id, id)
-        .visible
-        .exists?
-  end
-
   def import_latest_plaid_data
     PlaidItem::Importer.new(self, plaid_provider: plaid_provider).import
   end

app/models/sync.rb

@@ -29,13 +29,13 @@ class Sync < ApplicationRecord
     state :failed
     state :stale
 
-    after_all_transitions :log_status_change
+    after_all_transitions :handle_transition
 
-    event :start, after_commit: :report_warnings do
+    event :start, after_commit: :handle_start_transition do
       transitions from: :pending, to: :syncing
     end
 
-    event :complete do
+    event :complete, after_commit: :handle_completion_transition do
       transitions from: :syncing, to: :completed
     end
 
@@ -163,9 +163,30 @@ class Sync < ApplicationRecord
       end
     end
 
+    def handle_start_transition
+      report_warnings
+    end
+
+    def handle_transition
+      log_status_change
+      family.touch(:latest_sync_activity_at)
+    end
+
+    def handle_completion_transition
+      family.touch(:latest_sync_completed_at)
+    end
+
     def window_valid
       if window_start_date && window_end_date && window_start_date > window_end_date
         errors.add(:window_end_date, "must be greater than window_start_date")
       end
     end
+
+    def family
+      if syncable.is_a?(Family)
+        syncable
+      else
+        syncable.family
+      end
+    end
 end