Simplefin sync improvements (#240)

* Fix syncing issues with new connections and accounts..

- Keep SimpleFin institution metadata strictly per account (`simplefin_accounts.org_data`).
- Relax `simplefin_items` institution constraints to allow creating items before org data exists.
- Remove code that copied the first account’s `org` onto `simplefin_items`.

* Improve Simplefin Sync
•
SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs.
•
Stop populating item‑level institution fields; keep institution metadata per account.
•
Relax NOT NULL on item institution fields.
•
Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job).
•
Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited.
•
Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list.

* Fixed double‑quoted strings, spacing around array brackets and commas

* chore: ignore local .junie files

* - Broadcast error logs now include full backtraces
- SimpleFin discovery logic deduplicated fixed
- app/models/simplefin_item/importer.rb
--Added a concise docstring for perform_account_discovery describing purpose, steps, and side‑effects.
--Added a docstring for fetch_accounts_data describing params and return value.

app/models/family/sync_complete_event.rb

@@ -6,16 +6,27 @@ class Family::SyncCompleteEvent
   end
 
   def broadcast
-    family.broadcast_replace(
-      target: "balance-sheet",
-      partial: "pages/dashboard/balance_sheet",
-      locals: { balance_sheet: family.balance_sheet }
-    )
+    # Dashboard partials can occasionally raise when rendered from background jobs
+    # (e.g., if intermediate series values are nil during a sync). Make broadcasts
+    # resilient so a post-sync UI refresh never causes the overall sync to report an error.
+    begin
+      family.broadcast_replace(
+        target: "balance-sheet",
+        partial: "pages/dashboard/balance_sheet",
+        locals: { balance_sheet: family.balance_sheet }
+      )
+    rescue => e
+      Rails.logger.error("Family::SyncCompleteEvent balance_sheet broadcast failed: #{e.message}\n#{e.backtrace&.join("\n")}")
+    end
 
-    family.broadcast_replace(
-      target: "net-worth-chart",
-      partial: "pages/dashboard/net_worth_chart",
-      locals: { balance_sheet: family.balance_sheet, period: Period.last_30_days }
-    )
+    begin
+      family.broadcast_replace(
+        target: "net-worth-chart",
+        partial: "pages/dashboard/net_worth_chart",
+        locals: { balance_sheet: family.balance_sheet, period: Period.last_30_days }
+      )
+    rescue => e
+      Rails.logger.error("Family::SyncCompleteEvent net_worth_chart broadcast failed: #{e.message}\n#{e.backtrace&.join("\n")}")
+    end
   end
 end

app/models/family/syncer.rb

@@ -26,6 +26,6 @@ class Family::Syncer
 
   private
     def child_syncables
-      family.plaid_items + family.accounts.manual
+      family.plaid_items + family.simplefin_items.active + family.accounts.manual
     end
 end

app/models/simplefin_item.rb

@@ -54,13 +54,8 @@ class SimplefinItem < ApplicationRecord
       raw_payload: accounts_snapshot,
     )
 
-    # Extract institution data from the first account if available
-    snapshot = accounts_snapshot.to_h.with_indifferent_access
-    if snapshot[:accounts].present?
-      first_account = snapshot[:accounts].first
-      org = first_account[:org]
-      upsert_institution_data!(org) if org.present?
-    end
+    # Do not populate item-level institution fields from account data.
+    # Institution metadata belongs to each simplefin_account (in org_data).
 
     save!
   end
@@ -153,6 +148,26 @@ class SimplefinItem < ApplicationRecord
     end
   end
 
+  # Detect a recent rate-limited sync and return a friendly message, else nil
+  def rate_limited_message
+    latest = latest_sync
+    return nil unless latest
+
+    # Some Sync records may not have a status_text column; guard with respond_to?
+    parts = []
+    parts << latest.error if latest.respond_to?(:error)
+    parts << latest.status_text if latest.respond_to?(:status_text)
+    msg = parts.compact.join(" — ")
+    return nil if msg.blank?
+
+    down = msg.downcase
+    if down.include?("make fewer requests") || down.include?("only refreshed once every 24 hours") || down.include?("rate limit")
+      "You've hit SimpleFin's daily refresh limit. Please try again after the bridge refreshes (up to 24 hours)."
+    else
+      nil
+    end
+  end
+
   private
     def remove_simplefin_item
       # SimpleFin doesn't require server-side cleanup like Plaid

app/models/simplefin_item/importer.rb

@@ -1,4 +1,5 @@
 class SimplefinItem::Importer
+  class RateLimitedError < StandardError; end
   attr_reader :simplefin_item, :simplefin_provider
 
   def initialize(simplefin_item, simplefin_provider:)
@@ -44,6 +45,10 @@ class SimplefinItem::Importer
         target_start_date = max_lookback_date
       end
 
+      # Pre-step: Unbounded discovery to ensure we see all accounts even if the
+      # chunked window would otherwise filter out newly added, inactive accounts.
+      perform_account_discovery
+
       total_accounts_imported = 0
       chunk_count = 0
 
@@ -100,30 +105,75 @@ class SimplefinItem::Importer
     end
 
     def import_regular_sync
-      start_date = determine_sync_start_date
+      perform_account_discovery
 
+      # Step 2: Fetch transactions/holdings using the regular window.
+      start_date = determine_sync_start_date
       accounts_data = fetch_accounts_data(start_date: start_date)
       return if accounts_data.nil? # Error already handled
 
       # Store raw payload
       simplefin_item.upsert_simplefin_snapshot!(accounts_data)
 
-      # Import accounts
+      # Import accounts (merges transactions/holdings into existing rows)
       accounts_data[:accounts]&.each do |account_data|
         import_account(account_data)
       end
     end
 
-    def fetch_accounts_data(start_date:, end_date: nil)
+    #
+    # Performs discovery of accounts in an unbounded way so providers that
+    # filter by date windows cannot hide newly created upstream accounts.
+    #
+    # Steps:
+    # - Request `/accounts` without dates; count results
+    # - If zero, retry with `pending: true` (some bridges only reveal new/pending)
+    # - If any accounts are returned, upsert a snapshot and import each account
+    #
+    # Returns nothing; side-effects are snapshot + account upserts.
+    def perform_account_discovery
+      discovery_data = fetch_accounts_data(start_date: nil)
+      discovered_count = discovery_data&.dig(:accounts)&.size.to_i
+      Rails.logger.info "SimpleFin discovery (no params) returned #{discovered_count} accounts"
+
+      if discovered_count.zero?
+        discovery_data = fetch_accounts_data(start_date: nil, pending: true)
+        discovered_count = discovery_data&.dig(:accounts)&.size.to_i
+        Rails.logger.info "SimpleFin discovery (pending=1) returned #{discovered_count} accounts"
+      end
+
+      if discovery_data && discovered_count > 0
+        simplefin_item.upsert_simplefin_snapshot!(discovery_data)
+        discovery_data[:accounts]&.each { |account_data| import_account(account_data) }
+      end
+    end
+
+    # Fetches accounts (and optionally transactions/holdings) from SimpleFin.
+    #
+    # Params:
+    # - start_date: Date or nil — when provided, provider may filter by date window
+    # - end_date:   Date or nil — optional end of window
+    # - pending:    Boolean or nil — when true, ask provider to include pending/new
+    #
+    # Returns a Hash payload with keys like :accounts, or nil when an error is
+    # handled internally via `handle_errors`.
+    def fetch_accounts_data(start_date:, end_date: nil, pending: nil)
       # Debug logging to track exactly what's being sent to SimpleFin API
-      days_requested = end_date ? (end_date.to_date - start_date.to_date).to_i : "unknown"
-      Rails.logger.info "SimplefinItem::Importer - API Request: #{start_date.strftime('%Y-%m-%d')} to #{end_date&.strftime('%Y-%m-%d') || 'current'} (#{days_requested} days)"
+      start_str = start_date.respond_to?(:strftime) ? start_date.strftime("%Y-%m-%d") : "none"
+      end_str = end_date.respond_to?(:strftime) ? end_date.strftime("%Y-%m-%d") : "current"
+      days_requested = if start_date && end_date
+        (end_date.to_date - start_date.to_date).to_i
+      else
+        "unknown"
+      end
+      Rails.logger.info "SimplefinItem::Importer - API Request: #{start_str} to #{end_str} (#{days_requested} days)"
 
       begin
         accounts_data = simplefin_provider.get_accounts(
           simplefin_item.access_url,
           start_date: start_date,
-          end_date: end_date
+          end_date: end_date,
+          pending: pending
         )
       rescue Provider::Simplefin::SimplefinError => e
         # Handle authentication errors by marking item as requiring update
@@ -141,6 +191,12 @@ class SimplefinItem::Importer
         return nil
       end
 
+      # Some servers return a top-level message/string rather than an errors array
+      if accounts_data[:error].present?
+        handle_errors([ accounts_data[:error] ])
+        return nil
+      end
+
       accounts_data
     end
 
@@ -224,6 +280,13 @@ class SimplefinItem::Importer
         simplefin_item.update!(status: :requires_update)
       end
 
+      # Detect and surface rate-limit specifically with a friendlier exception
+      if error_messages.downcase.include?("make fewer requests") ||
+         error_messages.downcase.include?("only refreshed once every 24 hours") ||
+         error_messages.downcase.include?("rate limit")
+        raise RateLimitedError, "SimpleFin rate limit: data refreshes at most once every 24 hours. Try again later."
+      end
+
       raise Provider::Simplefin::SimplefinError.new(
         "SimpleFin API errors: #{error_messages}",
         :api_error