From f25fe30a414a5b7bc12a723f44837dbe3d1bc960 Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Tue, 9 Jun 2026 23:02:26 +0200 Subject: [PATCH] feat(ai): honor Setting.llm_provider for batch and PDF flows (#2265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auto-categorization, merchant detection/enhancement, and PDF/bank-statement extraction hard-coded Provider::Registry.get_provider(:openai), so selecting Anthropic (or running an Anthropic-only self-hosted install) left those operations using/missing OpenAI rather than the chosen provider. Add Provider::Registry.preferred_llm_provider, which resolves the LLM provider honoring Setting.llm_provider with a configured-provider fallback (mirroring how chat picks its provider), and route all six TODO(#2113) call sites through it: - Family::AutoCategorizer#llm_provider - Family::AutoMerchantDetector#llm_provider - ProviderMerchant::Enhancer#llm_provider - PdfImport (process_pdf + extract_bank_statement) - Assistant::Function::ImportBankStatement Provider::Anthropic already implements auto_categorize / auto_detect_merchants / enhance_provider_merchants (#1984) and process_pdf / extract_bank_statement (#1985), so no provider changes are needed — only the wiring. Closes #2113. --- .../function/import_bank_statement.rb | 10 +++--- app/models/family/auto_categorizer.rb | 10 +++--- app/models/family/auto_merchant_detector.rb | 11 +++--- app/models/pdf_import.rb | 12 +++---- app/models/provider/registry.rb | 14 ++++++++ app/models/provider_merchant/enhancer.rb | 11 +++--- test/models/provider/registry_test.rb | 35 +++++++++++++++++++ .../models/provider_merchant/enhancer_test.rb | 2 ++ 8 files changed, 76 insertions(+), 29 deletions(-) diff --git a/app/models/assistant/function/import_bank_statement.rb b/app/models/assistant/function/import_bank_statement.rb index d3bf86dec..8920ffad5 100644 --- a/app/models/assistant/function/import_bank_statement.rb +++ b/app/models/assistant/function/import_bank_statement.rb @@ -92,15 +92,15 @@ class Assistant::Function::ImportBankStatement < Assistant::Function } end - # Extract transactions from the PDF using provider - # TODO(#2113): hardcoded to OpenAI. Provider::Anthropic implements - # extract_bank_statement (PR #1985); this should honor Setting.llm_provider. - provider = Provider::Registry.get_provider(:openai) + # Extract transactions from the PDF using the configured LLM provider. + # Honors Setting.llm_provider (issue #2113) — Provider::Anthropic implements + # extract_bank_statement (PR #1985). + provider = Provider::Registry.preferred_llm_provider unless provider return { success: false, error: "provider_not_configured", - message: "OpenAI provider is not configured" + message: "AI provider is not configured" } end diff --git a/app/models/family/auto_categorizer.rb b/app/models/family/auto_categorizer.rb index c28cb7eb5..de56e18d5 100644 --- a/app/models/family/auto_categorizer.rb +++ b/app/models/family/auto_categorizer.rb @@ -58,13 +58,11 @@ class Family::AutoCategorizer private attr_reader :family, :transaction_ids - # TODO(#2113): hardcoded to OpenAI. Provider::Anthropic now - # implements auto_categorize (PR #1984), so this should honor - # Setting.llm_provider the way chat does, instead of always routing batch - # categorization to OpenAI. Until then, Anthropic batch ops are only - # reachable directly / via the eval runner, not this family flow. + # Honors Setting.llm_provider (issue #2113) — Provider::Anthropic implements + # auto_categorize (PR #1984), so batch categorization routes to the configured + # provider, with fallback handled by Provider::Registry.preferred_llm_provider. def llm_provider - Provider::Registry.get_provider(:openai) + Provider::Registry.preferred_llm_provider end def user_categories_input diff --git a/app/models/family/auto_merchant_detector.rb b/app/models/family/auto_merchant_detector.rb index e93da64b4..45ebabcbb 100644 --- a/app/models/family/auto_merchant_detector.rb +++ b/app/models/family/auto_merchant_detector.rb @@ -61,13 +61,12 @@ class Family::AutoMerchantDetector private attr_reader :family, :transaction_ids - # TODO(#2113): hardcoded to OpenAI. Provider::Anthropic now - # implements auto_detect_merchants (PR #1984), so this should honor - # Setting.llm_provider the way chat does, instead of always routing batch - # merchant detection to OpenAI. Until then, Anthropic batch ops are only - # reachable directly / via the eval runner, not this family flow. + # Honors Setting.llm_provider (issue #2113) — Provider::Anthropic implements + # auto_detect_merchants (PR #1984), so batch merchant detection routes to the + # configured provider, with fallback handled by + # Provider::Registry.preferred_llm_provider. def llm_provider - Provider::Registry.get_provider(:openai) + Provider::Registry.preferred_llm_provider end def default_logo_provider_url diff --git a/app/models/pdf_import.rb b/app/models/pdf_import.rb index 69c74cb9e..616b81644 100644 --- a/app/models/pdf_import.rb +++ b/app/models/pdf_import.rb @@ -89,9 +89,9 @@ class PdfImport < Import end def process_with_ai - # TODO(#2113): hardcoded to OpenAI. Provider::Anthropic implements - # process_pdf (PR #1985); this should honor Setting.llm_provider. - provider = Provider::Registry.get_provider(:openai) + # Honors Setting.llm_provider (issue #2113) — Provider::Anthropic implements + # process_pdf (PR #1985). + provider = Provider::Registry.preferred_llm_provider raise "AI provider not configured" unless provider raise "AI provider does not support PDF processing" unless provider.supports_pdf_processing? @@ -117,9 +117,9 @@ class PdfImport < Import def extract_transactions return unless statement_with_transactions? - # TODO(#2113): hardcoded to OpenAI. Provider::Anthropic implements - # extract_bank_statement (PR #1985); this should honor Setting.llm_provider. - provider = Provider::Registry.get_provider(:openai) + # Honors Setting.llm_provider (issue #2113) — Provider::Anthropic implements + # extract_bank_statement (PR #1985). + provider = Provider::Registry.preferred_llm_provider raise "AI provider not configured" unless provider response = provider.extract_bank_statement( diff --git a/app/models/provider/registry.rb b/app/models/provider/registry.rb index 085c31b50..b573e33a7 100644 --- a/app/models/provider/registry.rb +++ b/app/models/provider/registry.rb @@ -18,6 +18,20 @@ class Provider::Registry raise Error.new("Provider '#{name}' not found in registry") end + # Resolves the LLM provider for batch/PDF flows, honoring Setting.llm_provider + # the way chat does: prefer the configured provider, but fall back to whichever + # one actually has credentials so an install that swaps providers (or has only + # one configured) keeps working. Returns nil when neither is configured — + # callers guard on that. + def preferred_llm_provider + order = Setting.llm_provider == "anthropic" ? %i[anthropic openai] : %i[openai anthropic] + order.each do |name| + provider = get_provider(name) + return provider if provider + end + nil + end + def plaid_provider_for_region(region) region.to_sym == :us ? plaid_us : plaid_eu end diff --git a/app/models/provider_merchant/enhancer.rb b/app/models/provider_merchant/enhancer.rb index e13d141a7..7114e66fd 100644 --- a/app/models/provider_merchant/enhancer.rb +++ b/app/models/provider_merchant/enhancer.rb @@ -70,13 +70,12 @@ class ProviderMerchant::Enhancer count end - # TODO(#2113): hardcoded to OpenAI. Provider::Anthropic now - # implements enhance_provider_merchants (PR #1984), so this should honor - # Setting.llm_provider the way chat does, instead of always routing merchant - # enhancement to OpenAI. Until then, Anthropic batch ops are only reachable - # directly / via the eval runner, not this family flow. + # Honors Setting.llm_provider (issue #2113) — Provider::Anthropic implements + # enhance_provider_merchants (PR #1984), so merchant enhancement routes to the + # configured provider, with fallback handled by + # Provider::Registry.preferred_llm_provider. def llm_provider - @llm_provider ||= Provider::Registry.get_provider(:openai) + @llm_provider ||= Provider::Registry.preferred_llm_provider end def unenhanced_merchants diff --git a/test/models/provider/registry_test.rb b/test/models/provider/registry_test.rb index 2aa274662..48c3d2fdf 100644 --- a/test/models/provider/registry_test.rb +++ b/test/models/provider/registry_test.rb @@ -107,4 +107,39 @@ class Provider::RegistryTest < ActiveSupport::TestCase assert_instance_of Provider::Openai, provider end end + + test "preferred_llm_provider returns openai by default" do + openai = mock("openai") + Provider::Registry.stubs(:openai).returns(openai) + Provider::Registry.stubs(:anthropic).returns(mock("anthropic")) + Setting.stubs(:llm_provider).returns("openai") + + assert_same openai, Provider::Registry.preferred_llm_provider + end + + test "preferred_llm_provider returns anthropic when selected" do + anthropic = mock("anthropic") + Provider::Registry.stubs(:openai).returns(mock("openai")) + Provider::Registry.stubs(:anthropic).returns(anthropic) + Setting.stubs(:llm_provider).returns("anthropic") + + assert_same anthropic, Provider::Registry.preferred_llm_provider + end + + test "preferred_llm_provider falls back to the configured provider when the selected one is unconfigured" do + openai = mock("openai") + Provider::Registry.stubs(:openai).returns(openai) + Provider::Registry.stubs(:anthropic).returns(nil) + Setting.stubs(:llm_provider).returns("anthropic") + + assert_same openai, Provider::Registry.preferred_llm_provider + end + + test "preferred_llm_provider returns nil when no provider is configured" do + Provider::Registry.stubs(:openai).returns(nil) + Provider::Registry.stubs(:anthropic).returns(nil) + Setting.stubs(:llm_provider).returns("openai") + + assert_nil Provider::Registry.preferred_llm_provider + end end diff --git a/test/models/provider_merchant/enhancer_test.rb b/test/models/provider_merchant/enhancer_test.rb index e3d717a38..abd92d7e3 100644 --- a/test/models/provider_merchant/enhancer_test.rb +++ b/test/models/provider_merchant/enhancer_test.rb @@ -71,7 +71,9 @@ class ProviderMerchant::EnhancerTest < ActiveSupport::TestCase end test "returns zero counts when no LLM provider" do + # No provider configured: preferred_llm_provider checks both before nil. Provider::Registry.stubs(:get_provider).with(:openai).returns(nil) + Provider::Registry.stubs(:get_provider).with(:anthropic).returns(nil) result = ProviderMerchant::Enhancer.new(@family).enhance