diff --git a/app/controllers/family_merchants_controller.rb b/app/controllers/family_merchants_controller.rb index 9a72b8659..4d41db132 100644 --- a/app/controllers/family_merchants_controller.rb +++ b/app/controllers/family_merchants_controller.rb @@ -125,7 +125,6 @@ class FamilyMerchantsController < ApplicationController combined_ids = (family_merchant_ids + provider_merchant_ids).uniq Merchant.where(id: combined_ids) - .distinct .order(Arel.sql("LOWER(COALESCE(name, ''))")) end end diff --git a/app/models/provider/yahoo_finance.rb b/app/models/provider/yahoo_finance.rb index bb008b5f6..eff80232f 100644 --- a/app/models/provider/yahoo_finance.rb +++ b/app/models/provider/yahoo_finance.rb @@ -260,7 +260,7 @@ class Provider::YahooFinance < Provider prices << Price.new( symbol: symbol, - date: Time.at(timestamp).to_date, + date: Time.at(timestamp).utc.to_date, price: normalized_price, currency: normalized_currency, exchange_operating_mic: exchange_operating_mic @@ -390,7 +390,7 @@ class Provider::YahooFinance < Provider symbol = "#{from}#{to}=X" fetch_chart_data(symbol, start_date, end_date) do |timestamp, close_rate| Rate.new( - date: Time.at(timestamp).to_date, + date: Time.at(timestamp).utc.to_date, from: from, to: to, rate: close_rate.to_f @@ -402,7 +402,7 @@ class Provider::YahooFinance < Provider symbol = "#{to}#{from}=X" rates = fetch_chart_data(symbol, start_date, end_date) do |timestamp, close_rate| Rate.new( - date: Time.at(timestamp).to_date, + date: Time.at(timestamp).utc.to_date, from: from, to: to, rate: (1.0 / close_rate.to_f).round(8) diff --git a/app/models/security/price.rb b/app/models/security/price.rb index 2d2bdf7f3..4feb765a7 100644 --- a/app/models/security/price.rb +++ b/app/models/security/price.rb @@ -4,13 +4,12 @@ class Security::Price < ApplicationRecord validates :date, :price, :currency, presence: true validates :date, uniqueness: { scope: %i[security_id currency] } - # Provisional prices from recent weekdays that should be re-fetched + # Provisional prices from recent days that should be re-fetched # - Must be provisional (gap-filled) - # - Must be from the last few days (configurable, default 3) - # - Must be a weekday (Saturday = 6, Sunday = 0 in PostgreSQL DOW) - scope :refetchable_provisional, ->(lookback_days: 3) { + # - Must be from the last few days (configurable, default 7) + # - Includes weekends: they get fixed via cascade when weekday prices are fetched + scope :refetchable_provisional, ->(lookback_days: 7) { where(provisional: true) .where(date: lookback_days.days.ago.to_date..Date.current) - .where("EXTRACT(DOW FROM date) NOT IN (0, 6)") } end diff --git a/app/models/security/price/importer.rb b/app/models/security/price/importer.rb index 7d522fee5..def62fc03 100644 --- a/app/models/security/price/importer.rb +++ b/app/models/security/price/importer.rb @@ -2,7 +2,7 @@ class Security::Price::Importer MissingSecurityPriceError = Class.new(StandardError) MissingStartPriceError = Class.new(StandardError) - PROVISIONAL_LOOKBACK_DAYS = 3 + PROVISIONAL_LOOKBACK_DAYS = 7 def initialize(security:, security_provider:, start_date:, end_date:, clear_cache: false) @security = security @@ -26,6 +26,7 @@ class Security::Price::Importer end prev_price_value = start_price_value + prev_currency = prev_price_currency || db_price_currency || "USD" unless prev_price_value.present? Rails.logger.error("Could not find a start price for #{security.ticker} on or before #{start_date}") @@ -49,22 +50,32 @@ class Security::Price::Importer provider_currency = provider_price&.currency has_provider_price = provider_price_value.present? && provider_price_value.to_f > 0 + has_db_price = db_price_value.present? && db_price_value.to_f > 0 is_provisional = db_price&.provisional - chosen_price = if clear_cache || is_provisional - provider_price_value || db_price_value # overwrite when possible (or when provisional) + # Choose price and currency from the same source to avoid mismatches + chosen_price, chosen_currency = if clear_cache || is_provisional + # For provisional/cache clear: only use provider price, let gap-fill handle missing + # This ensures stale DB values don't persist when provider has no weekend data + [ provider_price_value, provider_currency ] + elsif has_db_price + # For non-provisional with valid DB price: preserve existing value (user edits) + [ db_price_value, db_price&.currency ] else - db_price_value || provider_price_value # fill gaps + # Fill gaps with provider data + [ provider_price_value, provider_currency ] end # Gap-fill using LOCF (last observation carried forward) - # Treat nil or zero prices as invalid and use previous price + # Treat nil or zero prices as invalid and use previous price/currency used_locf = false if chosen_price.nil? || chosen_price.to_f <= 0 chosen_price = prev_price_value + chosen_currency = prev_currency used_locf = true end prev_price_value = chosen_price + prev_currency = chosen_currency || prev_currency provisional = determine_provisional_status( date: date, @@ -77,7 +88,7 @@ class Security::Price::Importer security_id: security.id, date: date, price: chosen_price, - currency: provider_currency || prev_price_currency || db_price_currency || "USD", + currency: chosen_currency || "USD", provisional: provisional } end @@ -90,7 +101,7 @@ class Security::Price::Importer def provider_prices @provider_prices ||= begin - provider_fetch_start_date = effective_start_date - 5.days + provider_fetch_start_date = effective_start_date - PROVISIONAL_LOOKBACK_DAYS.days response = security_provider.fetch_security_prices( symbol: security.ticker, @@ -151,22 +162,53 @@ class Security::Price::Importer end def start_price_value - provider_price_value = provider_prices.select { |date, _| date <= start_date } - .max_by { |date, _| date } - &.last&.price - db_price_value = db_prices[start_date]&.price - provider_price_value || db_price_value + # When processing full range (first sync), use original behavior + if effective_start_date == start_date + provider_price_value = provider_prices.select { |date, _| date <= start_date } + .max_by { |date, _| date } + &.last&.price + db_price_value = db_prices[start_date]&.price + + return provider_price_value if provider_price_value.present? && provider_price_value.to_f > 0 + return db_price_value if db_price_value.present? && db_price_value.to_f > 0 + return nil + end + + # For partial range (effective_start_date > start_date), use recent data + # This prevents stale prices from old trade dates propagating to current gap-fills + cutoff_date = effective_start_date + + # First try provider prices (most recent before cutoff) + provider_price_value = provider_prices + .select { |date, _| date < cutoff_date } + .max_by { |date, _| date } + &.last&.price + + return provider_price_value if provider_price_value.present? && provider_price_value.to_f > 0 + + # Fall back to most recent DB price before cutoff + currency = prev_price_currency || db_price_currency + Security::Price + .where(security_id: security.id) + .where("date < ?", cutoff_date) + .where("price > 0") + .where(provisional: false) + .then { |q| currency.present? ? q.where(currency: currency) : q } + .order(date: :desc) + .limit(1) + .pick(:price) end def determine_provisional_status(date:, has_provider_price:, used_locf:, existing_provisional:) # Provider returned real price => NOT provisional return false if has_provider_price - # Gap-filled (LOCF) => provisional only if recent weekday + # Gap-filled (LOCF) => provisional if recent (including weekends) + # Weekend prices inherit uncertainty from Friday and get fixed via cascade + # when the next weekday sync fetches correct Friday price if used_locf - is_weekday = !date.saturday? && !date.sunday? is_recent = date >= PROVISIONAL_LOOKBACK_DAYS.days.ago.to_date - return is_weekday && is_recent + return is_recent end # Otherwise preserve existing status diff --git a/db/schema.rb b/db/schema.rb index 1672e0fc1..b73b0e6ae 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: 2026_01_09_144012) do +ActiveRecord::Schema[7.2].define(version: 2026_01_10_122603) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" diff --git a/test/models/account/market_data_importer_test.rb b/test/models/account/market_data_importer_test.rb index 4a305e94f..828f34814 100644 --- a/test/models/account/market_data_importer_test.rb +++ b/test/models/account/market_data_importer_test.rb @@ -4,7 +4,8 @@ require "ostruct" class Account::MarketDataImporterTest < ActiveSupport::TestCase include ProviderTestHelper - PROVIDER_BUFFER = 5.days + SECURITY_PRICE_BUFFER = Security::Price::Importer::PROVISIONAL_LOOKBACK_DAYS.days + EXCHANGE_RATE_BUFFER = 5.days setup do # Ensure a clean slate for deterministic assertions @@ -37,7 +38,7 @@ class Account::MarketDataImporterTest < ActiveSupport::TestCase ExchangeRate.create!(from_currency: "CAD", to_currency: "USD", date: existing_date, rate: 2.0) ExchangeRate.create!(from_currency: "USD", to_currency: "CAD", date: existing_date, rate: 0.5) - expected_start_date = (existing_date + 1.day) - PROVIDER_BUFFER + expected_start_date = (existing_date + 1.day) - EXCHANGE_RATE_BUFFER end_date = Date.current.in_time_zone("America/New_York").to_date @provider.expects(:fetch_exchange_rates) @@ -88,7 +89,7 @@ class Account::MarketDataImporterTest < ActiveSupport::TestCase entryable: trade ) - expected_start_date = trade_date - PROVIDER_BUFFER + expected_start_date = trade_date - SECURITY_PRICE_BUFFER end_date = Date.current.in_time_zone("America/New_York").to_date @provider.expects(:fetch_security_prices) @@ -138,7 +139,7 @@ class Account::MarketDataImporterTest < ActiveSupport::TestCase entryable: trade ) - expected_start_date = trade_date - PROVIDER_BUFFER + expected_start_date = trade_date - SECURITY_PRICE_BUFFER end_date = Date.current.in_time_zone("America/New_York").to_date # Simulate provider returning an error response @@ -181,7 +182,7 @@ class Account::MarketDataImporterTest < ActiveSupport::TestCase ExchangeRate.create!(from_currency: "CAD", to_currency: "USD", date: existing_date, rate: 2.0) ExchangeRate.create!(from_currency: "USD", to_currency: "CAD", date: existing_date, rate: 0.5) - expected_start_date = (existing_date + 1.day) - PROVIDER_BUFFER + expected_start_date = (existing_date + 1.day) - EXCHANGE_RATE_BUFFER end_date = Date.current.in_time_zone("America/New_York").to_date # Simulate provider returning an error response diff --git a/test/models/market_data_importer_test.rb b/test/models/market_data_importer_test.rb index 2a9ff3460..37d3070fe 100644 --- a/test/models/market_data_importer_test.rb +++ b/test/models/market_data_importer_test.rb @@ -4,8 +4,9 @@ require "ostruct" class MarketDataImporterTest < ActiveSupport::TestCase include ProviderTestHelper - SNAPSHOT_START_DATE = MarketDataImporter::SNAPSHOT_DAYS.days.ago.to_date - PROVIDER_BUFFER = 5.days + SNAPSHOT_START_DATE = MarketDataImporter::SNAPSHOT_DAYS.days.ago.to_date + SECURITY_PRICE_BUFFER = Security::Price::Importer::PROVISIONAL_LOOKBACK_DAYS.days + EXCHANGE_RATE_BUFFER = 5.days setup do Security::Price.delete_all @@ -39,7 +40,7 @@ class MarketDataImporterTest < ActiveSupport::TestCase date: SNAPSHOT_START_DATE, rate: 0.5) - expected_start_date = (SNAPSHOT_START_DATE + 1.day) - PROVIDER_BUFFER + expected_start_date = (SNAPSHOT_START_DATE + 1.day) - EXCHANGE_RATE_BUFFER end_date = Date.current.in_time_zone("America/New_York").to_date @provider.expects(:fetch_exchange_rates) @@ -70,7 +71,7 @@ class MarketDataImporterTest < ActiveSupport::TestCase test "syncs security prices" do security = Security.create!(ticker: "AAPL", exchange_operating_mic: "XNAS") - expected_start_date = SNAPSHOT_START_DATE - PROVIDER_BUFFER + expected_start_date = SNAPSHOT_START_DATE - SECURITY_PRICE_BUFFER end_date = Date.current.in_time_zone("America/New_York").to_date @provider.expects(:fetch_security_prices) diff --git a/test/models/security/price/importer_test.rb b/test/models/security/price/importer_test.rb index ed8697ffd..6a2fb8c42 100644 --- a/test/models/security/price/importer_test.rb +++ b/test/models/security/price/importer_test.rb @@ -160,7 +160,7 @@ class Security::Price::ImporterTest < ActiveSupport::TestCase assert db_prices.all? { |p| p.provisional == false }, "All prices from provider should not be provisional" end - test "marks gap-filled weekend prices as not provisional" do + test "marks gap-filled weekend prices as provisional" do Security::Price.delete_all # Find a recent Saturday @@ -186,7 +186,9 @@ class Security::Price::ImporterTest < ActiveSupport::TestCase ).import_provider_prices saturday_price = Security::Price.find_by(security: @security, date: saturday) - assert_not saturday_price.provisional, "Weekend gap-filled price should not be provisional" + # Weekend gap-filled prices are now provisional so they can be fixed + # via cascade when the next weekday sync fetches the correct Friday price + assert saturday_price.provisional, "Weekend gap-filled price should be provisional" end test "marks gap-filled recent weekday prices as provisional" do @@ -311,8 +313,107 @@ class Security::Price::ImporterTest < ActiveSupport::TestCase assert_not old_price.provisional, "Old gap-filled price should not be provisional" end + test "provisional weekend prices get fixed via cascade from Friday" do + Security::Price.delete_all + + # Find a recent Monday + monday = Date.current + monday += 1.day until monday.monday? + friday = monday - 3.days + saturday = monday - 2.days + sunday = monday - 1.day + + travel_to monday do + # Create provisional weekend prices with WRONG values (simulating stale data) + Security::Price.create!(security: @security, date: saturday, price: 50, currency: "USD", provisional: true) + Security::Price.create!(security: @security, date: sunday, price: 50, currency: "USD", provisional: true) + + # Provider returns Friday and Monday prices, but NOT weekend (markets closed) + provider_response = provider_success_response([ + OpenStruct.new(security: @security, date: friday, price: 150, currency: "USD"), + OpenStruct.new(security: @security, date: monday, price: 155, currency: "USD") + ]) + + @provider.expects(:fetch_security_prices).returns(provider_response) + + Security::Price::Importer.new( + security: @security, + security_provider: @provider, + start_date: friday, + end_date: monday + ).import_provider_prices + + # Friday should have real price from provider + friday_price = Security::Price.find_by(security: @security, date: friday) + assert_equal 150, friday_price.price + assert_not friday_price.provisional, "Friday should not be provisional (real price)" + + # Saturday should be gap-filled from Friday (150), not old wrong value (50) + saturday_price = Security::Price.find_by(security: @security, date: saturday) + assert_equal 150, saturday_price.price, "Saturday should use Friday's price via cascade" + assert saturday_price.provisional, "Saturday should be provisional (gap-filled)" + + # Sunday should be gap-filled from Saturday (150) + sunday_price = Security::Price.find_by(security: @security, date: sunday) + assert_equal 150, sunday_price.price, "Sunday should use Friday's price via cascade" + assert sunday_price.provisional, "Sunday should be provisional (gap-filled)" + + # Monday should have real price from provider + monday_price = Security::Price.find_by(security: @security, date: monday) + assert_equal 155, monday_price.price + assert_not monday_price.provisional, "Monday should not be provisional (real price)" + end + end + + test "uses recent prices for gap-fill when effective_start_date skips old dates" do + Security::Price.delete_all + + # Use travel_to to ensure we're on a weekday for consistent test behavior + # Find the next weekday if today is a weekend + test_date = Date.current + test_date += 1.day while test_date.saturday? || test_date.sunday? + + travel_to test_date do + # Simulate: old price exists from first trade date (30 days ago) with STALE value + old_date = 30.days.ago.to_date + stale_price = 50 + + # Fully populate DB from old_date through yesterday so effective_start_date = today + # Use stale price for old dates, then recent price for recent dates + (old_date..1.day.ago.to_date).each do |date| + # Use stale price for dates older than lookback window, recent price for recent dates + price = date < 7.days.ago.to_date ? stale_price : 150 + Security::Price.create!(security: @security, date: date, price: price, currency: "USD") + end + + # Provider returns yesterday's price (155) - DIFFERENT from DB (150) to prove we use provider + # Provider does NOT return today (simulating market closed) + provider_response = provider_success_response([ + OpenStruct.new(security: @security, date: 1.day.ago.to_date, price: 155, currency: "USD") + ]) + + @provider.expects(:fetch_security_prices).returns(provider_response) + + Security::Price::Importer.new( + security: @security, + security_provider: @provider, + start_date: old_date, + end_date: Date.current + ).import_provider_prices + + today_price = Security::Price.find_by(security: @security, date: Date.current) + + # effective_start_date should be today (only missing date) + # start_price_value should use provider's yesterday (155), not stale old DB price (50) + # Today should gap-fill from that recent price + assert_equal 155, today_price.price, "Gap-fill should use recent provider price, not stale old price" + # Should be provisional since gap-filled for recent weekday + assert today_price.provisional, "Current weekday gap-filled price should be provisional" + end + end + private def get_provider_fetch_start_date(start_date) - start_date - 5.days + start_date - Security::Price::Importer::PROVISIONAL_LOOKBACK_DAYS.days end end