From fea228d03e2c8b982b9915982ba2525866d907e1 Mon Sep 17 00:00:00 2001
From: luckyPipewrench <142104046+luckyPipewrench@users.noreply.github.com>
Date: Sun, 26 Oct 2025 10:50:45 -0400
Subject: [PATCH] Simplefin sync improvements (#240)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* 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.
---
.gitignore | 3 +
app/models/family/sync_complete_event.rb | 31 +++++---
app/models/family/syncer.rb | 2 +-
app/models/simplefin_item.rb | 29 +++++--
app/models/simplefin_item/importer.rb | 75 +++++++++++++++++--
.../simplefin_items/_simplefin_item.html.erb | 5 ++
..._simplefin_item_institution_constraints.rb | 20 +++++
db/schema.rb | 7 +-
lib/tasks/simplefin.rake | 35 +++++++++
9 files changed, 181 insertions(+), 26 deletions(-)
create mode 100644 db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb
create mode 100644 lib/tasks/simplefin.rake
diff --git a/.gitignore b/.gitignore
index 24423a7dc..4fbcee9d6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -106,3 +106,6 @@ scripts/
.windsurfrules
.cursor/rules/dev_workflow.mdc
.cursor/rules/taskmaster.mdc
+
+# Local Junie helpers
+.junie/
diff --git a/app/models/family/sync_complete_event.rb b/app/models/family/sync_complete_event.rb
index 628841d0e..4fd0e00f5 100644
--- a/app/models/family/sync_complete_event.rb
+++ b/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
diff --git a/app/models/family/syncer.rb b/app/models/family/syncer.rb
index 30ce2ad5e..2f120f81d 100644
--- a/app/models/family/syncer.rb
+++ b/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
diff --git a/app/models/simplefin_item.rb b/app/models/simplefin_item.rb
index 1e194f76b..4d4b9f08a 100644
--- a/app/models/simplefin_item.rb
+++ b/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
diff --git a/app/models/simplefin_item/importer.rb b/app/models/simplefin_item/importer.rb
index d17b7df2a..a4b11cccc 100644
--- a/app/models/simplefin_item/importer.rb
+++ b/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
diff --git a/app/views/simplefin_items/_simplefin_item.html.erb b/app/views/simplefin_items/_simplefin_item.html.erb
index 3f2768d3f..d63d4bd7a 100644
--- a/app/views/simplefin_items/_simplefin_item.html.erb
+++ b/app/views/simplefin_items/_simplefin_item.html.erb
@@ -38,6 +38,11 @@
<%= icon "alert-triangle", size: "sm", color: "warning" %>
<%= tag.span t(".requires_update") %>
+ <% elsif simplefin_item.rate_limited_message.present? %>
+
+ <%= icon "clock", size: "sm", color: "warning" %>
+ <%= tag.span simplefin_item.rate_limited_message %>
+
<% elsif simplefin_item.sync_error.present? %>
<%= render DS::Tooltip.new(text: simplefin_item.sync_error, icon: "alert-circle", size: "sm", color: "destructive") %>
diff --git a/db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb b/db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb
new file mode 100644
index 000000000..a935414a1
--- /dev/null
+++ b/db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb
@@ -0,0 +1,20 @@
+class RelaxSimplefinItemInstitutionConstraints < ActiveRecord::Migration[7.2]
+ def up
+ # SimpleFin doesn't guarantee institution metadata on first fetch,
+ # so these fields must be optional.
+ change_column_null :simplefin_items, :institution_id, true
+ change_column_null :simplefin_items, :institution_name, true
+ end
+
+ def down
+ # Restoring NOT NULL could break existing rows that legitimately have no institution metadata.
+ # We keep this reversible but conservative: only set NOT NULL if no NULLs exist.
+ if execute("SELECT COUNT(*) FROM simplefin_items WHERE institution_id IS NULL").first["count"].to_i == 0
+ change_column_null :simplefin_items, :institution_id, false
+ end
+
+ if execute("SELECT COUNT(*) FROM simplefin_items WHERE institution_name IS NULL").first["count"].to_i == 0
+ change_column_null :simplefin_items, :institution_name, false
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 1d97447db..5255d298e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.2].define(version: 2025_10_24_083624) do
+ActiveRecord::Schema[7.2].define(version: 2025_10_25_095800) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
@@ -29,7 +29,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_10_24_083624) do
t.uuid "accountable_id"
t.decimal "balance", precision: 19, scale: 4
t.string "currency"
- t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true
+ t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true
t.uuid "import_id"
t.uuid "plaid_account_id"
t.decimal "cash_balance", precision: 19, scale: 4, default: "0.0"
@@ -754,6 +754,9 @@ ActiveRecord::Schema[7.2].define(version: 2025_10_24_083624) do
t.string "institution_color"
t.date "sync_start_date"
t.index ["family_id"], name: "index_simplefin_items_on_family_id"
+ t.index ["institution_domain"], name: "index_simplefin_items_on_institution_domain"
+ t.index ["institution_id"], name: "index_simplefin_items_on_institution_id"
+ t.index ["institution_name"], name: "index_simplefin_items_on_institution_name"
t.index ["status"], name: "index_simplefin_items_on_status"
end
diff --git a/lib/tasks/simplefin.rake b/lib/tasks/simplefin.rake
new file mode 100644
index 000000000..df4d3fe09
--- /dev/null
+++ b/lib/tasks/simplefin.rake
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+namespace :sure do
+ namespace :simplefin do
+ desc "Print debug info for a SimpleFin item: latest sync, snapshot accounts, simplefin_accounts, and unlinked list"
+ task :debug, [ :item_id ] => :environment do |_, args|
+ unless args[:item_id].present?
+ puts({ error: "usage", example: "bin/rails sure:simplefin:debug[ITEM_ID]" }.to_json)
+ exit 1
+ end
+
+ item = SimplefinItem.find(args[:item_id])
+ latest_sync = item.syncs.order(created_at: :desc).first
+ # Our model stores the latest snapshot directly on the item (`raw_payload`).
+ snapshot_accounts = item.raw_payload&.dig(:accounts)&.size
+ unlinked = item.simplefin_accounts.left_joins(:account).where(accounts: { id: nil })
+
+ out = {
+ item_id: item.id,
+ name: item.name,
+ last_synced_at: item.last_synced_at,
+ latest_sync: latest_sync&.attributes&.slice("id", "status", "error", "status_text", "created_at", "completed_at"),
+ snapshot_accounts: snapshot_accounts,
+ simplefin_accounts_count: item.simplefin_accounts.count,
+ unlinked_count: unlinked.count,
+ unlinked: unlinked.limit(20).map { |sfa| { id: sfa.id, upstream_id: sfa.account_id, name: sfa.name } }
+ }
+
+ puts out.to_json
+ rescue => e
+ puts({ error: e.class.name, message: e.message, backtrace: e.backtrace&.take(3) }.to_json)
+ exit 1
+ end
+ end
+end