diff --git a/app/models/assistant.rb b/app/models/assistant.rb index 1cb432d61..1635b67b5 100644 --- a/app/models/assistant.rb +++ b/app/models/assistant.rb @@ -8,13 +8,17 @@ module Assistant # Statement Vault + provenance tools, for users who opted into preview features # in Settings -> Preferences. They back the wealth agent-harness workflow - # documented in docs/llm-guides/wealth-agent-harness.md. + # documented in docs/llm-guides/wealth-agent-harness.md. GetValuations is the + # read pair for RecordValuation; GetInsights reads the Insights feed, which is + # itself preview-gated app-wide. PREVIEW_FUNCTION_CLASSES = [ Function::UploadAccountStatement, Function::ListAccountStatements, Function::GetAccountStatement, Function::GetStatementCoverage, - Function::RecordValuation + Function::RecordValuation, + Function::GetValuations, + Function::GetInsights ].freeze class << self @@ -38,6 +42,7 @@ module Assistant def function_classes(user = nil) classes = [ Function::GetTransactions, + Function::GetRecurringTransactions, Function::GetAccounts, Function::GetHoldings, Function::GetBalanceSheet, @@ -52,6 +57,7 @@ module Assistant Function::GetCategories, Function::CreateCategory, Function::UpdateCategory, + Function::GetMerchants, Function::UpdateTransaction, Function::UpdateBudget ] diff --git a/app/models/assistant/configurable.rb b/app/models/assistant/configurable.rb index d50bbd318..1c16229ce 100644 --- a/app/models/assistant/configurable.rb +++ b/app/models/assistant/configurable.rb @@ -1,6 +1,53 @@ module Assistant::Configurable extend ActiveSupport::Concern + # The byte-stable half of the system prompt. Everything volatile (date, + # currency, per-family context) lives in the trailing Session context block, + # because providers cache and discount an exactly-repeated prefix; one + # changed byte mid-prompt invalidates everything after it. Self-hosters + # customizing the prompt should edit this constant. + STATIC_INSTRUCTIONS = <<~PROMPT.freeze + ## Your identity + + You are a friendly financial assistant for an open source personal finance application called "Sure", which is short for "Sure Finances". + + ## Your purpose + + You help users understand their financial data by answering questions about their accounts, transactions, income, expenses, net worth, budgets, forecasting and more. + + ## How to handle a request + + First classify the request, then act: + + - CHAT: greetings, thanks, or general personal-finance concepts. Answer directly. Do not call tools. + - LOOKUP: one specific fact (a balance, a total, one transaction). Call the single most specific tool, then answer. + - ANALYSIS: trends, comparisons, or multi-part questions. Decide which tools you need before calling any; make independent calls in the same round. + + ### Tool rules + + - Reuse data already present in this conversation or in the Session context below instead of calling a tool again for it. Exception: always re-fetch when the data may have changed (for example after you created or updated something) or when the user asks for a different time range or more detail. + - Prefer the most specific tool: use get_income_statement or get_balance_sheet for totals and trends; use get_transactions only to find or inspect individual transactions. + - If a tool result contains an "error" and a "hint", follow the hint and retry once with corrected arguments. Never repeat an identical failing call. + - If you suspect that you do not have enough data to 100% accurately answer, be transparent about it and state exactly what the data you're presenting represents and what context it is in (i.e. date range, account, etc.) + + ### Response rules + + - Provide ONLY the most important numbers and insights + - Eliminate all unnecessary words and context + - Ask follow-up questions to keep the conversation going. Help educate the user about their own data and entice them to ask more questions. + - Do NOT add introductions or conclusions + - Do NOT apologize or explain limitations + - Format all responses in markdown + - Format monetary values in the user's preferred currency and dates in the user's preferred format, both given in Session context below. When no currency is specified, use the preferred currency. + + ### Rules about financial advice + + You should focus on educating the user about personal finance using their own data so they can make informed decisions. + + - Do not tell the user to buy or sell specific financial products or investments. + - Do not make assumptions about the user's financial situation. Use the functions available to get the data you need. + PROMPT + class_methods do def config_for(chat) preferred_currency = Money::Currency.new(chat.user.family.currency) @@ -13,7 +60,7 @@ module Assistant::Configurable } else { - instructions: default_instructions(preferred_currency, preferred_date_format), + instructions: default_instructions(preferred_currency, preferred_date_format, user: chat.user), functions: default_functions(chat.user) } end @@ -55,61 +102,77 @@ module Assistant::Configurable Assistant.function_classes(user) end - def default_instructions(preferred_currency, preferred_date_format) + def default_instructions(preferred_currency, preferred_date_format, user: nil) + "#{Assistant::Configurable::STATIC_INSTRUCTIONS}\n#{session_context(preferred_currency, preferred_date_format, user: user)}" + end + + def session_context(preferred_currency, preferred_date_format, user: nil) <<~PROMPT - ## Your identity + ## Session context - You are a friendly financial assistant for an open source personal finance application called "Sure", which is short for "Sure Finances". - - ## Your purpose - - You help users understand their financial data by answering questions about their accounts, transactions, income, expenses, net worth, forecasting and more. - - ## Your rules - - Follow all rules below at all times. - - ### General rules - - - Provide ONLY the most important numbers and insights - - Eliminate all unnecessary words and context - - Ask follow-up questions to keep the conversation going. Help educate the user about their own data and entice them to ask more questions. - - Do NOT add introductions or conclusions - - Do NOT apologize or explain limitations - - ### Formatting rules - - - Format all responses in markdown - - Format all monetary values according to the user's preferred currency - - Format dates in the user's preferred format: #{preferred_date_format} - - #### User's preferred currency - - Sure is a multi-currency app where each user has a "preferred currency" setting. - - When no currency is specified, use the user's preferred currency for formatting and displaying monetary values. - - - Symbol: #{preferred_currency.symbol} - - ISO code: #{preferred_currency.iso_code} - - Default precision: #{preferred_currency.default_precision} - - Default format: #{preferred_currency.default_format} - - Separator: #{preferred_currency.separator} - - Delimiter: #{preferred_currency.delimiter} - - ### Rules about financial advice - - You should focus on educating the user about personal finance using their own data so they can make informed decisions. - - - Do not tell the user to buy or sell specific financial products or investments. - - Do not make assumptions about the user's financial situation. Use the functions available to get the data you need. - - ### Function calling rules - - - Use the functions available to you to get user financial data and enhance your responses - - For functions that require dates, use the current date as your reference point: #{Date.current} - - If you suspect that you do not have enough data to 100% accurately answer, be transparent about it and state exactly what - the data you're presenting represents and what context it is in (i.e. date range, account, etc.) + - Today's date: #{Date.current}. For functions that require dates, use it as your reference point. + - Date format: #{preferred_date_format} + - Preferred currency: #{preferred_currency.iso_code} (symbol #{preferred_currency.symbol}, precision #{preferred_currency.default_precision}, format #{preferred_currency.default_format}, separator "#{preferred_currency.separator}", delimiter "#{preferred_currency.delimiter}") + #{accounts_context(user)}#{categories_context(user)} PROMPT end + + # One line per account, from columns already loaded. Collapses to counts + # for large families or small model context windows so the volatile tail + # of the prompt stays cheap. + ACCOUNTS_ROSTER_LIMIT = 25 + CATEGORY_NAMES_LIMIT = 60 + + def accounts_context(user) + return "" if user.nil? + + accounts = user.accessible_accounts.visible.to_a + return "" if accounts.empty? + + if accounts.size > ACCOUNTS_ROSTER_LIMIT || Assistant::TokenBudget.small_context? + counts = accounts.group_by(&:accountable_type).map { |type, group| "#{group.size} #{type}" }.join(", ") + + <<~CONTEXT + + ### Accounts + + #{accounts.size} accounts: #{counts}. Call get_accounts for the list. + CONTEXT + else + lines = accounts.map do |account| + "- #{account.name}: #{account.accountable_type}, #{account.classification}, #{account.balance_money.format}" + end + + <<~CONTEXT + + ### Accounts + + #{lines.join("\n")} + CONTEXT + end + end + + def categories_context(user) + return "" if user.nil? + + names = user.family.categories.pluck(:name) + return "" if names.empty? + + if names.size > CATEGORY_NAMES_LIMIT || Assistant::TokenBudget.small_context? + <<~CONTEXT + + ### Categories + + #{names.size} categories. Call get_categories for the list. + CONTEXT + else + <<~CONTEXT + + ### Categories + + #{(names + [ "Uncategorized" ]).join(", ")} + CONTEXT + end + end end end diff --git a/app/models/assistant/function.rb b/app/models/assistant/function.rb index c2d6eac60..43bfebc99 100644 --- a/app/models/assistant/function.rb +++ b/app/models/assistant/function.rb @@ -9,6 +9,8 @@ class Assistant::Function end end + MAX_PAGE_SIZE = 100 + def initialize(user) @user = user end @@ -91,26 +93,27 @@ class Assistant::Function end end - def family_account_names - @family_account_names ||= user.accessible_accounts.visible.pluck(:name) - end - - def family_category_names - @family_category_names ||= begin - names = family.categories.pluck(:name) - names << "Uncategorized" - names - end - end - - def family_merchant_names - @family_merchant_names ||= family.merchants.pluck(:name) - end - + # Still used by update_tag, which identifies tags by name. Tag lists are + # small; the large data-driven enums (accounts, categories, merchants) + # are gone from schemas in favor of exact-name params. def family_tag_names @family_tag_names ||= family.tags.pluck(:name) end + # Shared page-size clamp for paginated tools declaring a page_size param. + def resolved_page_size(params) + return self.class.default_page_size if params["page_size"].blank? + + params["page_size"].to_i.clamp(1, MAX_PAGE_SIZE) + end + + # Pagy raises on zero, negative or non-numeric pages; normalize anything + # invalid to the first page instead of failing the call. + def resolved_page(params) + page = params["page"].to_i + page.positive? ? page : 1 + end + def family user.family end @@ -119,14 +122,21 @@ class Assistant::Function UuidFormat.valid?(str) end - # To save tokens, we provide the AI metadata about the series and a flat array of - # raw, formatted values which it can infer dates from + # To save tokens, we provide the AI metadata about the series and a flat + # array of raw numeric values it can infer dates from. Currency is stated + # once here instead of formatting every value; the system prompt's + # formatting rules cover rendering. Values round to the currency's own + # precision (BTC carries 8 decimals, OMR 3), never a flat 2. def to_ai_time_series(series) + currency = series.values.first&.trend&.current&.currency + precision = currency&.default_precision || 2 + { start_date: series.start_date, end_date: series.end_date, interval: series.interval, - values: series.values.map { |v| v.trend.current.format } - } + currency: currency&.iso_code, + values: series.values.map { |v| v.trend.current.amount.round(precision).to_f } + }.compact end end diff --git a/app/models/assistant/function/get_accounts.rb b/app/models/assistant/function/get_accounts.rb index 13706d215..692078cf4 100644 --- a/app/models/assistant/function/get_accounts.rb +++ b/app/models/assistant/function/get_accounts.rb @@ -5,15 +5,47 @@ class Assistant::Function::GetAccounts < Assistant::Function end def description - "Use this to see what accounts the user has along with their current and historical balances" + <<~INSTRUCTIONS + Use this to see what accounts the user has along with their current balances. + + Returns account ids. Use them for account_ids filters in other tools. + + Pass include_balance_series: true only when the user asks about balance + history; the series is omitted by default to keep responses small. + INSTRUCTIONS end end + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + include_balance_series: { + type: "boolean", + description: "Include a historical balance series per account (defaults to false)" + }, + series_period: { + type: "string", + enum: Period::PERIODS.keys, + description: "Period for the balance series (defaults to last_365_days)" + } + } + ) + end + def call(params = {}) + include_series = params["include_balance_series"] == true + period = series_period(params) + { as_of_date: Date.current, - accounts: user.accessible_accounts.includes(:balances, :account_providers).map do |account| - { + accounts: accounts_scope(include_series).map do |account| + payload = { + id: account.id, name: account.name, balance: account.balance, currency: account.currency, @@ -23,19 +55,41 @@ class Assistant::Function::GetAccounts < Assistant::Function start_date: account.start_date, is_linked: account.linked?, provider: account.provider_name, - status: account.status, - historical_balances: historical_balances(account) + status: account.status } + + if include_series + series = historical_balances(account, period) + payload[:historical_balances] = series if series + end + payload end } end private - def historical_balances(account) - start_date = [ account.start_date, 5.years.ago.to_date ].max - period = Period.custom(start_date: start_date, end_date: Date.current) - balance_series = account.balance_series(period: period, interval: "1 month") + # No balances preload: the series goes through Balance::ChartSeriesBuilder, + # which runs its own query keyed by account ids. + def accounts_scope(_include_series) + user.accessible_accounts.visible.includes(:account_providers) + end + + def historical_balances(account, period) + effective_start = [ account.start_date, period.start_date ].max + # An account whose start date lies beyond the period (start_date derives + # from the first entry, which can be future-dated) simply has no series; + # it must not fail the whole accounts listing. + return nil if effective_start > period.end_date + + effective = Period.custom(start_date: effective_start, end_date: period.end_date) + balance_series = account.balance_series(period: effective, interval: effective.interval) to_ai_time_series(balance_series) end + + def series_period(params) + key = params["series_period"].to_s + + Period.valid_key?(key) ? Period.from_key(key) : Period.from_key("last_365_days") + end end diff --git a/app/models/assistant/function/get_balance_sheet.rb b/app/models/assistant/function/get_balance_sheet.rb index 1d8846876..709360f40 100644 --- a/app/models/assistant/function/get_balance_sheet.rb +++ b/app/models/assistant/function/get_balance_sheet.rb @@ -1,6 +1,9 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function include ActiveSupport::NumberHelper + MAX_SERIES_POINTS = 400 + INTERVALS = [ "1 day", "1 week", "1 month" ].freeze + class << self def name "get_balance_sheet" @@ -13,37 +16,104 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function This is great for answering questions like: - What is the user's net worth? What is it composed of? - How has the user's wealth changed over time? + + For "net worth over time" questions, pass a named period (or a custom + start_date and end_date) and an interval to control the granularity of + the history series. The default is the last 5 years at 1 month. INSTRUCTIONS end end - def call(params = {}) - observation_start_date = [ 5.years.ago.to_date, family.oldest_entry_date ].max + def strict_mode? + false + end - period = Period.custom(start_date: observation_start_date, end_date: Date.current) + def params_schema + build_schema( + required: [], + properties: { + period: { + type: "string", + enum: Period::PERIODS.keys, + description: "Named period for the history series (defaults to the last 5 years)" + }, + start_date: { + type: "string", + description: "Custom range start in YYYY-MM-DD format; overrides period when end_date is also given" + }, + end_date: { + type: "string", + description: "Custom range end in YYYY-MM-DD format; overrides period when start_date is also given" + }, + interval: { + type: "string", + enum: INTERVALS, + description: "Series granularity (defaults to 1 month)" + } + } + ) + end + + def call(params = {}) + period = resolve_period(params) + return period if period.is_a?(Hash) + + interval = params["interval"].presence_in(INTERVALS) || "1 month" + + if series_points(period, interval) > MAX_SERIES_POINTS + return error("too_many_points", "That period and interval combination produces too many data points. Use a coarser interval or a shorter period.") + end { as_of_date: Date.current, oldest_account_start_date: family.oldest_entry_date, currency: family.currency, net_worth: { - current: family.balance_sheet(user: user).net_worth_money.format, - monthly_history: historical_data(period) + current: balance_sheet.net_worth_money.format, + monthly_history: historical_data(period, interval) }, assets: { - current: family.balance_sheet(user: user).assets.total_money.format, - monthly_history: historical_data(period, classification: "asset") + current: balance_sheet.assets.total_money.format, + monthly_history: historical_data(period, interval, classification: "asset") }, liabilities: { - current: family.balance_sheet(user: user).liabilities.total_money.format, - monthly_history: historical_data(period, classification: "liability") + current: balance_sheet.liabilities.total_money.format, + monthly_history: historical_data(period, interval, classification: "liability") }, insights: insights_data } end private - def historical_data(period, classification: nil) + def balance_sheet + @balance_sheet ||= family.balance_sheet(user: user) + end + + def resolve_period(params) + if params["start_date"].present? && params["end_date"].present? + Period.custom( + start_date: Date.parse(params["start_date"]), + end_date: Date.parse(params["end_date"]) + ) + elsif Period.valid_key?(params["period"].to_s) + Period.from_key(params["period"]) + else + observation_start_date = [ 5.years.ago.to_date, family.oldest_entry_date ].max + Period.custom(start_date: observation_start_date, end_date: Date.current) + end + rescue Date::Error + error("invalid_date", "Dates must be valid and in YYYY-MM-DD format.") + rescue ActiveModel::ValidationError + # Period validates the range itself (e.g. start after end) + error("invalid_date", "start_date must be on or before end_date.") + end + + def series_points(period, interval) + days_per_point = { "1 day" => 1, "1 week" => 7, "1 month" => 30 }.fetch(interval) + period.days / days_per_point + end + + def historical_data(period, interval, classification: nil) scope = user.accessible_accounts.visible scope = scope.where(classification: classification) if classification.present? @@ -57,7 +127,7 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function currency: family.currency, period: period, favorable_direction: "up", - interval: "1 month" + interval: interval ) to_ai_time_series(builder.balance_series) @@ -65,12 +135,16 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function end def insights_data - assets = family.balance_sheet(user: user).assets.total - liabilities = family.balance_sheet(user: user).liabilities.total + assets = balance_sheet.assets.total + liabilities = balance_sheet.liabilities.total ratio = liabilities.zero? ? 0 : (liabilities / assets.to_f) { debt_to_asset_ratio: number_to_percentage(ratio * 100, precision: 0) } end + + def error(key, message) + { error: key, message: message } + end end diff --git a/app/models/assistant/function/get_categories.rb b/app/models/assistant/function/get_categories.rb index 57e39b089..189edfeb9 100644 --- a/app/models/assistant/function/get_categories.rb +++ b/app/models/assistant/function/get_categories.rb @@ -22,19 +22,32 @@ class Assistant::Function::GetCategories < Assistant::Function - `total_pages`: The total number of pages of results - `page`: The current page of results - - `page_size`: The number of results per page (this will always be #{default_page_size}) + - `page_size`: The number of results per page (defaults to #{default_page_size}) - `total_results`: The total number of results INSTRUCTIONS end end + # Optional params are incompatible with strict function calling, which + # requires every declared property to be listed in `required`. + def strict_mode? + false + end + def params_schema build_schema( required: [], properties: { page: { type: "integer", + minimum: 1, description: "Page number (defaults to 1)" + }, + page_size: { + type: "integer", + minimum: 1, + maximum: MAX_PAGE_SIZE, + description: "Results per page (defaults to #{self.class.default_page_size})" } } ) @@ -42,7 +55,8 @@ class Assistant::Function::GetCategories < Assistant::Function def call(params = {}) categories_scope = family.categories.alphabetically_by_hierarchy - pagy = Pagy.new(count: categories_scope.count, page: params["page"] || 1, limit: default_page_size) + page_size = resolved_page_size(params) + pagy = Pagy.new(count: categories_scope.count, page: resolved_page(params), limit: page_size) categories = categories_scope.offset(pagy.offset).limit(pagy.limit) { @@ -59,13 +73,8 @@ class Assistant::Function::GetCategories < Assistant::Function }, total_results: pagy.count, page: pagy.page, - page_size: default_page_size, + page_size: page_size, total_pages: pagy.pages } end - - private - def default_page_size - self.class.default_page_size - end end diff --git a/app/models/assistant/function/get_holdings.rb b/app/models/assistant/function/get_holdings.rb index c65769eb7..05d7330b9 100644 --- a/app/models/assistant/function/get_holdings.rb +++ b/app/models/assistant/function/get_holdings.rb @@ -62,6 +62,7 @@ class Assistant::Function::GetHoldings < Assistant::Function properties: { page: { type: "integer", + minimum: 1, description: "Page number" }, accounts: { @@ -86,7 +87,7 @@ class Assistant::Function::GetHoldings < Assistant::Function holdings_query = build_holdings_query(params) ordered_holdings = holdings_query.order(amount: :desc) - pagy = Pagy.new(count: ordered_holdings.count, page: params["page"] || 1, limit: default_page_size) + pagy = Pagy.new(count: ordered_holdings.count, page: resolved_page(params), limit: default_page_size) paginated_holdings = ordered_holdings.includes(:security, :account).offset(pagy.offset).limit(pagy.limit) total_value = holdings_query.sum(:amount) diff --git a/app/models/assistant/function/get_income_statement.rb b/app/models/assistant/function/get_income_statement.rb index ba3100a9e..2c9b1b80c 100644 --- a/app/models/assistant/function/get_income_statement.rb +++ b/app/models/assistant/function/get_income_statement.rb @@ -15,6 +15,13 @@ class Assistant::Function::GetIncomeStatement < Assistant::Function - What are the user's spending habits? - How much income or spending did the user have over a specific time period? + Spending trends and comparisons: + - Month over month: pass group_by: "month" for a monthly_series + (calendar months, unlike get_budget which honors a custom month start) + - Versus the prior period: pass compare_previous_period: true + - Per account: pass account_ids from get_accounts (totals only; the + category breakdown is family-wide and is omitted with this filter) + Simple example: ``` @@ -27,27 +34,50 @@ class Assistant::Function::GetIncomeStatement < Assistant::Function end end + MAX_MONTH_BUCKETS = 36 + + def strict_mode? + false + end + def call(params = {}) period = Period.custom(start_date: Date.parse(params["start_date"]), end_date: Date.parse(params["end_date"])) - income_data = family.income_statement.income_totals(period: period) - expense_data = family.income_statement.expense_totals(period: period) - { - currency: family.currency, - period: { - start_date: period.start_date, - end_date: period.end_date - }, - income: { - total: format_money(income_data.total), - by_category: to_ai_category_totals(income_data.category_totals) - }, - expense: { - total: format_money(expense_data.total), - by_category: to_ai_category_totals(expense_data.category_totals) - }, - insights: get_insights(income_data, expense_data) - } + account_ids = params["account_ids"].presence + if account_ids + # Validated against the accounts income/expense totals actually reflect: + # hidden, excluded-from-reports and tax-advantaged accounts would come + # back as silent zeros if accepted here. + eligible = income_statement.eligible_accounts.where(id: account_ids) + unknown_ids = account_ids.uniq - eligible.pluck(:id) + + if unknown_ids.any? + return { + error: "unknown_account_ids", + message: "Some account ids were not found or are not part of income and expense reporting (hidden, excluded-from-reports and tax-advantaged accounts are not eligible). Call get_accounts for ids and retry once with eligible accounts.", + unknown_ids: unknown_ids + } + end + end + + # Validate the bucket count before running any aggregation work + buckets = params["group_by"] == "month" ? month_buckets(period) : nil + + if buckets && buckets.size > MAX_MONTH_BUCKETS + return { + error: "too_many_periods", + message: "That range produces more than #{MAX_MONTH_BUCKETS} monthly buckets. Use a shorter range." + } + end + + result = account_ids ? scoped_result(period, account_ids) : full_result(period) + result[:monthly_series] = buckets.map { |bucket| bucket_totals(bucket, account_ids) } if buckets + + result[:previous_period] = previous_period_comparison(period, account_ids) if params["compare_previous_period"] + + result + rescue Date::Error + { error: "invalid_date", message: "Dates must be valid and in YYYY-MM-DD format." } end def params_schema @@ -61,12 +91,131 @@ class Assistant::Function::GetIncomeStatement < Assistant::Function end_date: { type: "string", description: "End date for aggregation period in YYYY-MM-DD format" + }, + account_ids: { + type: "array", + description: "Account UUIDs from get_accounts; scopes totals to those accounts and omits the category breakdown", + items: { type: "string" }, + minItems: 1, + uniqueItems: true + }, + group_by: { + type: "string", + enum: [ "none", "month" ], + description: "Pass \"month\" to add a monthly_series of income/expenses/net per calendar month" + }, + compare_previous_period: { + type: "boolean", + description: "Adds totals for the equal-length period immediately before start_date, with deltas" } } ) end private + # Scoped to the requesting user, like get_balance_sheet's. In the assistant + # and MCP paths there is no session, so Current.user is nil and an unscoped + # IncomeStatement silently reports family-wide totals. + def income_statement + @income_statement ||= family.income_statement(user: user) + end + + def full_result(period) + income_data = income_statement.income_totals(period: period) + expense_data = income_statement.expense_totals(period: period) + + { + currency: family.currency, + period: { + start_date: period.start_date, + end_date: period.end_date + }, + income: { + total: format_money(income_data.total), + by_category: to_ai_category_totals(income_data.category_totals) + }, + expense: { + total: format_money(expense_data.total), + by_category: to_ai_category_totals(expense_data.category_totals) + }, + insights: get_insights(income_data, expense_data) + } + end + + # Category rollups and family stats are family-wide by construction, so a + # per-account view reports totals only and says why the breakdown is gone. + def scoped_result(period, account_ids) + totals = income_statement.totals_for(period, account_ids: account_ids) + + { + currency: family.currency, + period: { + start_date: period.start_date, + end_date: period.end_date + }, + account_ids: account_ids, + income: { total: format_money(totals.income_money.amount), by_category: nil }, + expense: { total: format_money(totals.expense_money.amount), by_category: nil }, + net: format_money(totals.income_money.amount - totals.expense_money.amount), + breakdown_omitted_reason: "category breakdown is not available with an account filter" + } + end + + def month_buckets(period) + cursor = period.start_date + + [].tap do |buckets| + while cursor <= period.end_date + bucket_end = [ cursor.end_of_month, period.end_date ].min + buckets << Period.custom(start_date: cursor, end_date: bucket_end) + cursor = bucket_end + 1.day + end + end + end + + def bucket_totals(bucket, account_ids) + totals = income_statement.totals_for(bucket, account_ids: account_ids) + income = totals.income_money.amount + expenses = totals.expense_money.amount + + { + start_date: bucket.start_date, + end_date: bucket.end_date, + income: format_money(income), + expenses: format_money(expenses), + net: format_money(income - expenses) + } + end + + def previous_period_comparison(period, account_ids) + previous = Period.custom( + start_date: period.start_date - period.days.days, + end_date: period.start_date - 1.day + ) + current_totals = income_statement.totals_for(period, account_ids: account_ids) + previous_totals = income_statement.totals_for(previous, account_ids: account_ids) + + { + start_date: previous.start_date, + end_date: previous.end_date, + income: format_money(previous_totals.income_money.amount), + expenses: format_money(previous_totals.expense_money.amount), + net: format_money(previous_totals.income_money.amount - previous_totals.expense_money.amount), + income_change: change_stats(previous_totals.income_money.amount, current_totals.income_money.amount), + expenses_change: change_stats(previous_totals.expense_money.amount, current_totals.expense_money.amount) + } + end + + def change_stats(previous_amount, current_amount) + delta = current_amount - previous_amount + percent = previous_amount.zero? ? nil : ((delta / previous_amount.to_f) * 100).round(1) + + { + amount: format_money(delta), + percent: percent + } + end + def format_money(value) Money.new(value, family.currency).format end @@ -110,9 +259,9 @@ class Assistant::Function::GetIncomeStatement < Assistant::Function def get_insights(income_data, expense_data) net_income = income_data.total - expense_data.total savings_rate = calculate_savings_rate(income_data.total, expense_data.total) - median_monthly_income = family.income_statement.median_income - median_monthly_expenses = family.income_statement.median_expense - avg_monthly_expenses = family.income_statement.avg_expense + median_monthly_income = income_statement.median_income + median_monthly_expenses = income_statement.median_expense + avg_monthly_expenses = income_statement.avg_expense { net_income: format_money(net_income), diff --git a/app/models/assistant/function/get_insights.rb b/app/models/assistant/function/get_insights.rb new file mode 100644 index 000000000..16508ab4c --- /dev/null +++ b/app/models/assistant/function/get_insights.rb @@ -0,0 +1,79 @@ +class Assistant::Function::GetInsights < Assistant::Function + DEFAULT_LIMIT = 10 + MAX_LIMIT = 50 + + class << self + def name + "get_insights" + end + + def description + <<~INSTRUCTIONS + Reads the family's proactive insights feed: typed observations like spending + anomalies, cash-flow warnings, subscription audits, savings-rate changes and + net-worth milestones, generated nightly with pre-computed numbers in + `metadata`. + + Use this to answer "anything I should know about my finances?" or to ground + analysis in signals the app has already detected. Read-only: it does not + mark insights read or acknowledged. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + insight_type: { + type: "string", + enum: Insight::TYPES, + description: "Only return insights of this type" + }, + include_acknowledged: { + type: "boolean", + description: "Include insights the user has already acknowledged (defaults to false)" + }, + limit: { + type: "integer", + minimum: 1, + maximum: MAX_LIMIT, + description: "Max results (defaults to #{DEFAULT_LIMIT})" + } + } + ) + end + + # Insights are family-scoped by design (the nightly generator runs per + # family, not per user), so there is no per-account visibility filter here. + # That matches the web feed exactly: InsightsController serves + # Current.family.insights to every member, so this tool exposes nothing the + # /insights page does not already show the same user. + def call(params = {}) + scope = params["include_acknowledged"] ? family.insights.where.not(status: :expired) : family.insights.visible + scope = scope.where(insight_type: params["insight_type"]) if params["insight_type"].present? + + limit = params["limit"].present? ? params["limit"].to_i.clamp(1, MAX_LIMIT) : DEFAULT_LIMIT + + { + insights: scope.ordered.limit(limit).map { |insight| + { + id: insight.id, + type: insight.insight_type, + title: insight.title, + body: insight.body, + priority: insight.priority, + status: insight.status, + period_start: insight.period_start, + period_end: insight.period_end, + generated_at: insight.generated_at.iso8601, + metadata: insight.metadata + }.compact + } + } + end +end diff --git a/app/models/assistant/function/get_merchants.rb b/app/models/assistant/function/get_merchants.rb new file mode 100644 index 000000000..b354d02b4 --- /dev/null +++ b/app/models/assistant/function/get_merchants.rb @@ -0,0 +1,78 @@ +class Assistant::Function::GetMerchants < Assistant::Function + class << self + def default_page_size + 50 + end + + def name + "get_merchants" + end + + def description + <<~INSTRUCTIONS + Returns merchants relevant to the user's transactions, sorted alphabetically, + with pagination. Each entry includes the stable id needed for + update_transaction's merchant_id and the exact name usable in + get_transactions' merchants filter. + + Pass `search` to filter by name instead of paging through everything. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + page: { + type: "integer", + minimum: 1, + description: "Page number (defaults to 1)" + }, + page_size: { + type: "integer", + minimum: 1, + maximum: MAX_PAGE_SIZE, + description: "Results per page (defaults to #{self.class.default_page_size})" + }, + search: { + type: "string", + description: "Case-insensitive substring filter on merchant name" + } + } + ) + end + + def call(params = {}) + # available_merchants_for scopes to merchants on transactions in accounts + # this user can access (plus the family's own merchants), so merchants + # seen only in accounts hidden from the user never leak into the list. + scope = family.available_merchants_for(user).alphabetically + + if params["search"].present? + scope = scope.where("merchants.name ILIKE ?", "%#{ActiveRecord::Base.sanitize_sql_like(params["search"])}%") + end + + page_size = resolved_page_size(params) + pagy = Pagy.new(count: scope.count, page: resolved_page(params), limit: page_size) + merchants = scope.offset(pagy.offset).limit(pagy.limit) + + { + merchants: merchants.map { |m| + { + id: m.id, + name: m.name, + source: m.type == "FamilyMerchant" ? "family" : "provider" + } + }, + total_results: pagy.count, + page: pagy.page, + page_size: page_size, + total_pages: pagy.pages + } + end +end diff --git a/app/models/assistant/function/get_recurring_transactions.rb b/app/models/assistant/function/get_recurring_transactions.rb new file mode 100644 index 000000000..5defc776c --- /dev/null +++ b/app/models/assistant/function/get_recurring_transactions.rb @@ -0,0 +1,115 @@ +class Assistant::Function::GetRecurringTransactions < Assistant::Function + MAX_RESULTS = 200 + + class << self + def name + "get_recurring_transactions" + end + + def description + <<~INSTRUCTIONS + Lists detected and manual recurring transactions (subscriptions, salaries, + recurring bills and transfers) with expected amounts and next expected dates. + + Great for questions like: What subscriptions am I paying for? What bills are + coming up this month? How much recurring spend do I have? + + `status` defaults to "active". Pass `upcoming_within_days` to only see items + expected between today and that many days from now (overdue items appear + when no window is given). totals_by_currency sums active items excluding + transfers, which move money between the user's own accounts. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + status: { + type: "string", + enum: [ "active", "inactive", "all" ], + description: "Filter by status (defaults to active)" + }, + upcoming_within_days: { + type: "integer", + minimum: 1, + maximum: 365, + description: "Only include items whose next expected date falls within this many days" + } + } + ) + end + + def call(params = {}) + scope = family.recurring_transactions + .accessible_by(user) + .includes(:merchant, :account, :destination_account) + + status = params["status"].presence_in([ "active", "inactive", "all" ]) || "active" + scope = scope.where(status: status) unless status == "all" + + if params["upcoming_within_days"].present? + days = params["upcoming_within_days"].to_i.clamp(1, 365) + # A forward-looking window: overdue items (past next_expected_date) + # appear in unwindowed calls, not inside "the next N days". + scope = scope.where(next_expected_date: Date.current..days.days.from_now.to_date) + end + + total_count = scope.count + rows = scope.order(status: :asc, next_expected_date: :asc).limit(MAX_RESULTS).to_a + + { + as_of_date: Date.current, + total_results: total_count, + truncated: total_count > MAX_RESULTS, + recurring_transactions: rows.map { |rt| serialize(rt) }, + totals_by_currency: totals_by_currency(scope) + } + end + + private + def serialize(recurring) + { + id: recurring.id, + name: recurring.name || recurring.merchant&.name, + amount: recurring.amount_money.format, + expected_amount_range: expected_amount_range(recurring), + currency: recurring.currency, + status: recurring.status, + expected_day_of_month: recurring.expected_day_of_month, + next_expected_date: recurring.next_expected_date, + last_occurrence_date: recurring.last_occurrence_date, + occurrence_count: recurring.occurrence_count, + is_manual: recurring.manual?, + is_transfer: recurring.transfer?, + account: account_ref(recurring.account), + destination_account: account_ref(recurring.destination_account) + }.compact + end + + def account_ref(account) + return nil if account.nil? + + { id: account.id, name: account.name } + end + + def expected_amount_range(recurring) + [ recurring.expected_amount_min_money&.format, recurring.expected_amount_max_money&.format ].compact.presence + end + + # Transfers are excluded: money moving between the user's own accounts is + # not recurring spend, however regular it is. Summed over the full + # filtered scope (not the displayed rows) so a truncated list never + # reports a partial sum as the total. + def totals_by_currency(scope) + scope.where(status: "active", destination_account_id: nil) + .group(:currency) + .sum(:amount) + .each_with_object({}) { |(currency, sum), totals| totals[currency] = Money.new(sum, currency).format } + end +end diff --git a/app/models/assistant/function/get_tags.rb b/app/models/assistant/function/get_tags.rb index 7fd1d74b8..f5fbd56e2 100644 --- a/app/models/assistant/function/get_tags.rb +++ b/app/models/assistant/function/get_tags.rb @@ -21,19 +21,32 @@ class Assistant::Function::GetTags < Assistant::Function - `total_pages`: The total number of pages of results - `page`: The current page of results - - `page_size`: The number of results per page (this will always be #{default_page_size}) + - `page_size`: The number of results per page (defaults to #{default_page_size}) - `total_results`: The total number of results INSTRUCTIONS end end + # Optional params are incompatible with strict function calling, which + # requires every declared property to be listed in `required`. + def strict_mode? + false + end + def params_schema build_schema( required: [], properties: { page: { type: "integer", + minimum: 1, description: "Page number (defaults to 1)" + }, + page_size: { + type: "integer", + minimum: 1, + maximum: MAX_PAGE_SIZE, + description: "Results per page (defaults to #{self.class.default_page_size})" } } ) @@ -41,20 +54,16 @@ class Assistant::Function::GetTags < Assistant::Function def call(params = {}) tags_scope = family.tags.alphabetically - pagy = Pagy.new(count: tags_scope.count, page: params["page"] || 1, limit: default_page_size) + page_size = resolved_page_size(params) + pagy = Pagy.new(count: tags_scope.count, page: resolved_page(params), limit: page_size) tags = tags_scope.offset(pagy.offset).limit(pagy.limit) { tags: tags.map { |t| { id: t.id, name: t.name, color: t.color } }, total_results: pagy.count, page: pagy.page, - page_size: default_page_size, + page_size: page_size, total_pages: pagy.pages } end - - private - def default_page_size - self.class.default_page_size - end end diff --git a/app/models/assistant/function/get_transactions.rb b/app/models/assistant/function/get_transactions.rb index ec6a9ce4c..f873852c4 100644 --- a/app/models/assistant/function/get_transactions.rb +++ b/app/models/assistant/function/get_transactions.rb @@ -19,43 +19,21 @@ class Assistant::Function::GetTransactions < Assistant::Function This function is not great for: - Large time periods (use the get_income_statement function for this) + Filters take exact names: use the values returned by get_accounts, + get_categories, get_merchants, and get_tags when unsure. Pass + types: ["income", "expense"] to exclude transfers between the user's + own accounts. Use a small page_size when you only need a few rows. + Note on pagination: This function can be paginated. You can expect the following properties in the response: - `total_pages`: The total number of pages of results - `page`: The current page of results - - `page_size`: The number of results per page (this will always be #{default_page_size}) + - `page_size`: The number of results per page (defaults to #{default_page_size}) - `total_results`: The total number of results for the given filters - `total_income`: The total income for the given filters - `total_expenses`: The total expenses for the given filters - - Simple example (transactions from the last 30 days): - - ``` - get_transactions({ - page: 1, - start_date: "#{30.days.ago.to_date}", - end_date: "#{Date.current}" - }) - ``` - - More complex example (various filters): - - ``` - get_transactions({ - page: 1, - search: "mcdonalds", - accounts: ["Checking", "Savings"], - start_date: "#{30.days.ago.to_date}", - end_date: "#{Date.current}", - categories: ["Restaurants"], - merchants: ["McDonald's"], - tags: ["Food"], - amount: "100", - amount_operator: "less" - }) - ``` INSTRUCTIONS end end @@ -66,15 +44,28 @@ class Assistant::Function::GetTransactions < Assistant::Function def params_schema build_schema( - required: [ "order", "page" ], + required: [], properties: { page: { type: "integer", - description: "Page number" + minimum: 1, + description: "Page number (defaults to 1)" + }, + page_size: { + type: "integer", + minimum: 1, + maximum: MAX_PAGE_SIZE, + description: "Results per page (defaults to #{self.class.default_page_size}); use small values to save tokens" }, order: { + type: "string", enum: [ "asc", "desc" ], - description: "Order of the transactions by date" + description: "Sort direction (defaults to desc)" + }, + sort_by: { + type: "string", + enum: [ "date", "amount" ], + description: "Sort by date (default) or by absolute amount" }, search: { type: "string", @@ -97,31 +88,52 @@ class Assistant::Function::GetTransactions < Assistant::Function type: "string", description: "End date for transactions in YYYY-MM-DD format" }, + types: { + type: "array", + description: "Filter by kind; [\"income\", \"expense\"] excludes transfers between the user's own accounts", + items: { enum: [ "income", "expense", "transfer" ] }, + minItems: 1, + uniqueItems: true + }, + statuses: { + type: "array", + description: "Filter by status", + items: { enum: [ "pending", "confirmed" ] }, + minItems: 1, + uniqueItems: true + }, + account_ids: { + type: "array", + description: "Filter by account UUIDs as returned by get_accounts", + items: { type: "string" }, + minItems: 1, + uniqueItems: true + }, accounts: { type: "array", - description: "Filter transactions by account name", - items: { enum: family_account_names }, + description: "Filter by exact account names as returned by get_accounts", + items: { type: "string" }, minItems: 1, uniqueItems: true }, categories: { type: "array", - description: "Filter transactions by category name", - items: { enum: family_category_names }, + description: "Filter by exact category names as returned by get_categories (\"Uncategorized\" is accepted)", + items: { type: "string" }, minItems: 1, uniqueItems: true }, merchants: { type: "array", - description: "Filter transactions by merchant name", - items: { enum: family_merchant_names }, + description: "Filter by exact merchant names as returned by get_merchants", + items: { type: "string" }, minItems: 1, uniqueItems: true }, tags: { type: "array", - description: "Filter transactions by tag name", - items: { enum: family_tag_names }, + description: "Filter by exact tag names as returned by get_tags", + items: { type: "string" }, minItems: 1, uniqueItems: true } @@ -130,7 +142,8 @@ class Assistant::Function::GetTransactions < Assistant::Function end def call(params = {}) - search_params = params.except("order", "page") + search_params = params.except("order", "page", "page_size", "sort_by") + search_params["status"] = search_params.delete("statuses") if search_params.key?("statuses") search = Transaction::Search.new( family, @@ -138,10 +151,11 @@ class Assistant::Function::GetTransactions < Assistant::Function accessible_account_ids: user.accessible_accounts.visible.pluck(:id) ) transactions_query = search.transactions_scope - pagy_query = params["order"] == "asc" ? transactions_query.chronological : transactions_query.reverse_chronological + pagy_query = ordered(transactions_query, params) # By default, we give a small page size to force the AI to use filters effectively and save on tokens - pagy = Pagy.new(count: pagy_query.count, page: params["page"] || 1, limit: default_page_size) + page_size = resolved_page_size(params) + pagy = Pagy.new(count: pagy_query.count, page: resolved_page(params), limit: page_size) paginated_transactions = pagy_query.includes( { entry: :account }, :category, :merchant, :tags, @@ -174,7 +188,7 @@ class Assistant::Function::GetTransactions < Assistant::Function transactions: normalized_transactions, total_results: pagy.count, page: pagy.page, - page_size: default_page_size, + page_size: page_size, total_pages: pagy.pages, total_income: totals.income_money.format, total_expenses: totals.expense_money.format @@ -182,7 +196,16 @@ class Assistant::Function::GetTransactions < Assistant::Function end private - def default_page_size - self.class.default_page_size + def ordered(query, params) + if params["sort_by"] == "amount" + # Fully literal order strings; nothing user-provided reaches Arel.sql + if params["order"] == "asc" + query.order(Arel.sql("ABS(entries.amount) ASC"), Arel.sql("entries.date DESC")) + else + query.order(Arel.sql("ABS(entries.amount) DESC"), Arel.sql("entries.date DESC")) + end + else + params["order"] == "asc" ? query.chronological : query.reverse_chronological + end end end diff --git a/app/models/assistant/function/get_valuations.rb b/app/models/assistant/function/get_valuations.rb new file mode 100644 index 000000000..21dab76b1 --- /dev/null +++ b/app/models/assistant/function/get_valuations.rb @@ -0,0 +1,112 @@ +class Assistant::Function::GetValuations < Assistant::Function + class << self + def default_page_size + 50 + end + + def name + "get_valuations" + end + + def description + <<~INSTRUCTIONS + Lists recorded account valuations (reconciliations and anchors), newest first. + Each entry includes the provenance citation stored in its notes. Use this to + audit what record_valuation has written, find dates that already carry a + value, or trace where a balance number came from. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + account_id: { + type: "string", + description: "Only valuations for this account UUID (from get_accounts)" + }, + start_date: { + type: "string", + description: "Only valuations on or after this date (YYYY-MM-DD)" + }, + end_date: { + type: "string", + description: "Only valuations on or before this date (YYYY-MM-DD)" + }, + page: { + type: "integer", + minimum: 1, + description: "Page number (defaults to 1)" + } + } + ) + end + + def call(params = {}) + accessible_ids = user.accessible_accounts.visible.select(:id) + scope = family.entries + .where(entryable_type: "Valuation", account_id: accessible_ids) + .includes(:account, :entryable) + + if params["account_id"].present? + return error("invalid_account_id", "account_id must be a UUID from get_accounts.") unless valid_uuid?(params["account_id"]) + + scope = scope.where(account_id: params["account_id"]) + end + + start_date = parse_date(params["start_date"]) + end_date = parse_date(params["end_date"]) + + # A malformed date must fail loudly; silently dropping the filter would + # present unfiltered data as though the requested range was honored. + if start_date == :invalid || end_date == :invalid + return error("invalid_date", "Dates must be valid and in YYYY-MM-DD format.") + end + + if start_date && end_date && start_date > end_date + return error("invalid_date", "start_date must be on or before end_date.") + end + + scope = scope.where(date: start_date..) if start_date + scope = scope.where(date: ..end_date) if end_date + + ordered = scope.reverse_chronological + pagy = Pagy.new(count: ordered.count, page: resolved_page(params), limit: self.class.default_page_size) + + { + valuations: ordered.offset(pagy.offset).limit(pagy.limit).map { |entry| + { + entry_id: entry.id, + account: { id: entry.account_id, name: entry.account.name, currency: entry.account.currency }, + date: entry.date, + amount: entry.amount.to_s, + amount_formatted: entry.amount_money.format, + kind: entry.entryable.kind, + notes: entry.notes + } + }, + total_results: pagy.count, + page: pagy.page, + page_size: self.class.default_page_size, + total_pages: pagy.pages + } + end + + private + def parse_date(value) + return nil if value.blank? + + Date.iso8601(value.to_s) + rescue Date::Error + :invalid + end + + def error(key, message) + { error: key, message: message } + end +end diff --git a/app/models/assistant/function_tool_caller.rb b/app/models/assistant/function_tool_caller.rb index bc73b9c50..3faaa5cfd 100644 --- a/app/models/assistant/function_tool_caller.rb +++ b/app/models/assistant/function_tool_caller.rb @@ -21,14 +21,43 @@ class Assistant::FunctionToolCaller end private + # Tool failures come back as data instead of raising, so one bad call no + # longer aborts the whole turn. The hint steers the model toward a single + # corrected retry (the system prompt pairs it with a retry-once rule). def execute(function_request) fn = find_function(function_request) + + if fn.nil? + return { + error: "Unknown tool: #{function_request.function_name}", + hint: "Only call tools from the provided list." + } + end + fn_args = JSON.parse(function_request.function_args.presence || "{}") fn.call(fn_args) + rescue JSON::ParserError + { + error: "Arguments were not valid JSON", + hint: "Re-send #{function_request.function_name} with valid JSON arguments." + } + rescue ActiveRecord::RecordNotFound => e + { + error: e.message, + hint: "That record was not found. List valid options first (for example get_accounts or get_categories) and retry once with an exact match." + } + rescue Date::Error, ArgumentError, KeyError => e + { + error: e.message, + hint: "Check argument formats (dates are YYYY-MM-DD) and retry once with corrected arguments." + } rescue => e - raise FunctionExecutionError.new( - "Error calling function #{fn.name} with arguments #{fn_args}: #{e.message}" - ) + Rails.logger.error("Assistant tool #{fn.name} failed: #{e.class}: #{e.message}") + + { + error: "#{fn.name} failed unexpectedly", + hint: "Do not retry with the same arguments. Answer with the data you already have and note the gap." + } end def find_function(function_request) diff --git a/app/models/assistant/history_trimmer.rb b/app/models/assistant/history_trimmer.rb index 02e64f8b9..94be7a1d3 100644 --- a/app/models/assistant/history_trimmer.rb +++ b/app/models/assistant/history_trimmer.rb @@ -10,9 +10,11 @@ class Assistant::HistoryTrimmer kept = [] tokens = 0 - group_tool_pairs(@messages).reverse_each do |group| + group_tool_pairs(@messages).reverse_each.with_index do |group, index| group_tokens = Assistant::TokenEstimator.estimate(group) - break if tokens + group_tokens > @max_tokens + # The newest group always survives: dropping it would discard the very + # message the model is being asked to answer. + break if index.positive? && tokens + group_tokens > @max_tokens kept.unshift(*group) tokens += group_tokens diff --git a/app/models/assistant/responder.rb b/app/models/assistant/responder.rb index 674111f33..95fe4c2c2 100644 --- a/app/models/assistant/responder.rb +++ b/app/models/assistant/responder.rb @@ -1,7 +1,11 @@ class Assistant::Responder ToolCallLimitError = Class.new(StandardError) EmptyResponseError = Class.new(StandardError) - DEFAULT_MAX_TOOL_CALL_ITERATIONS = 5 + # Rounds, not calls: parallel calls in one round count once. Eight covers a + # taxonomy lookup, a target lookup, an action, one hint-corrected retry, a + # verification read and a summary, with margin. Override with + # ASSISTANT_MAX_TOOL_CALL_ITERATIONS (low-resource hosts may want 2). + DEFAULT_MAX_TOOL_CALL_ITERATIONS = 8 def initialize(message:, instructions:, function_tool_caller:, llm:) @message = message @@ -36,9 +40,18 @@ class Assistant::Responder function_tool_calls: function_tool_calls }) + # On the final permitted round the follow-up request forbids further + # tool calls via tool_choice, so the model must answer in text with + # whatever it has gathered instead of the turn dying in + # ToolCallLimitError. The real tool definitions stay in the request — + # Anthropic rejects messages containing tool blocks when no tools are + # defined, so dropping the tool list is not an option. + final_round = iteration == max_tool_call_iterations + response, response_has_text = request_response( function_results: provider_preserves_response_context? ? function_results : in_flight_function_results.dup, - previous_response_id: response.id + previous_response_id: response.id, + tool_choice: final_round ? :none : nil ) any_response_has_text ||= response_has_text end @@ -51,7 +64,7 @@ class Assistant::Responder private attr_reader :message, :instructions, :function_tool_caller, :llm - def request_response(function_results: [], previous_response_id: nil) + def request_response(function_results: [], previous_response_id: nil, tool_choice: nil) response_has_text = false streamer = proc do |chunk| @@ -64,7 +77,8 @@ class Assistant::Responder response = get_llm_response( streamer: streamer, function_results: function_results, - previous_response_id: previous_response_id + previous_response_id: previous_response_id, + tool_choice: tool_choice ) response_has_text ||= response.messages.any? { |response_message| response_message.output_text.present? } @@ -82,13 +96,14 @@ class Assistant::Responder DEFAULT_MAX_TOOL_CALL_ITERATIONS end - def get_llm_response(streamer:, function_results: [], previous_response_id: nil) + def get_llm_response(streamer:, function_results: [], previous_response_id: nil, tool_choice: nil) response = llm.chat_response( message.content, model: message.ai_model, instructions: instructions, functions: function_tool_caller.function_definitions, function_results: function_results, + tool_choice: tool_choice, messages: openai_messages_payload, conversation_history: chat_message_records, streamer: streamer, diff --git a/app/models/assistant/token_budget.rb b/app/models/assistant/token_budget.rb new file mode 100644 index 000000000..caf57ea45 --- /dev/null +++ b/app/models/assistant/token_budget.rb @@ -0,0 +1,26 @@ +module Assistant::TokenBudget + module_function + + # Shared resolution of the model's context window so prompt assembly and the + # provider layer agree on the same number. Precedence: ENV > Setting > + # default, matching Provider::Openai's historical behavior. + DEFAULT_CONTEXT_WINDOW = 2048 + + def context_window + from_env = ENV["LLM_CONTEXT_WINDOW"].to_s.strip.to_i + return from_env if from_env.positive? + + from_setting = Setting.llm_context_window.to_i + return from_setting if from_setting.positive? + + DEFAULT_CONTEXT_WINDOW + end + + # Below this, per-request context like account rosters is collapsed to + # counts so the volatile tail of the system prompt stays small. + SMALL_CONTEXT_THRESHOLD = 4096 + + def small_context? + context_window < SMALL_CONTEXT_THRESHOLD + end +end diff --git a/app/models/eval/runners/chat_runner.rb b/app/models/eval/runners/chat_runner.rb index a30d7b657..c33ad2cde 100644 --- a/app/models/eval/runners/chat_runner.rb +++ b/app/models/eval/runners/chat_runner.rb @@ -225,31 +225,40 @@ class Eval::Runners::ChatRunner < Eval::Runners::Base end end + # The real static prompt plus a fixed synthetic session context, so evals + # measure the prompt users actually run. The previous hardcoded stand-in + # (four fake permissive tool schemas, its own instructions) scored a + # fiction that could pass while the shipped prompt regressed. def build_instructions - # Simple instructions for evaluation - we don't have a real user/family context <<~PROMPT - You are a financial assistant helping users understand their financial data. - Use the functions available to answer questions about accounts, transactions, and financial statements. - Today's date is #{Date.current}. - PROMPT + #{Assistant::Configurable::STATIC_INSTRUCTIONS} + ## Session context + + - Today's date: #{Date.current}. For functions that require dates, use it as your reference point. + - Date format: %m-%d-%Y + - Preferred currency: USD (symbol $, precision 2, format %u%n, separator ".", delimiter ",") + PROMPT end + # Real definitions from the registry. Most schemas are user-independent + # now, but a few (get_holdings, update_tag) still embed small family + # scoped enums, so definitions build against a reference user; a class + # whose schema cannot build without one is skipped with a log line rather + # than silently faked. def build_function_definitions - # Return the function definitions that the chat would normally have - [ - build_function_definition("get_transactions", "Get paginated transactions with optional filters"), - build_function_definition("get_accounts", "Get all accounts with balances and historical data"), - build_function_definition("get_balance_sheet", "Get current net worth, assets, and liabilities"), - build_function_definition("get_income_statement", "Get income and expenses by category for a period") - ] + user = reference_user + + Assistant.function_classes(user).filter_map do |fn_class| + begin + fn_class.new(user).to_definition + rescue StandardError => e + log_progress("Skipping #{fn_class.name} definition: #{e.message}") + nil + end + end end - def build_function_definition(name, description) - { - name: name, - description: description, - params_schema: { type: "object", properties: {}, additionalProperties: true }, - strict: false - } + def reference_user + @reference_user ||= User.order(:created_at).first end end diff --git a/app/models/provider/anthropic.rb b/app/models/provider/anthropic.rb index ab05a0e0b..9328266ff 100644 --- a/app/models/provider/anthropic.rb +++ b/app/models/provider/anthropic.rb @@ -207,6 +207,7 @@ class Provider::Anthropic < Provider instructions: nil, functions: [], function_results: [], + tool_choice: nil, messages: nil, conversation_history: [], streamer: nil, @@ -221,6 +222,7 @@ class Provider::Anthropic < Provider instructions: instructions, functions: functions, function_results: function_results, + tool_choice: tool_choice, conversation_history: conversation_history, default_max_tokens: default_max_tokens ) diff --git a/app/models/provider/anthropic/chat_config.rb b/app/models/provider/anthropic/chat_config.rb index a4b3f2d08..a251c6374 100644 --- a/app/models/provider/anthropic/chat_config.rb +++ b/app/models/provider/anthropic/chat_config.rb @@ -4,6 +4,7 @@ class Provider::Anthropic::ChatConfig instructions: nil, functions: [], function_results: [], + tool_choice: nil, conversation_history: [], default_max_tokens: 4096 ) @@ -11,6 +12,7 @@ class Provider::Anthropic::ChatConfig @instructions = instructions @functions = functions @function_results = function_results + @tool_choice = tool_choice @conversation_history = conversation_history @default_max_tokens = default_max_tokens end @@ -26,7 +28,13 @@ class Provider::Anthropic::ChatConfig params[:system_] = system_blocks if system_blocks.present? tool_blocks = build_tools - params[:tools] = tool_blocks if tool_blocks.present? + if tool_blocks.present? + params[:tools] = tool_blocks + # Forbidding further tool calls still requires sending the tool + # definitions — the API rejects messages containing tool_use/tool_result + # blocks when no tools are defined. + params[:tool_choice] = { type: "none" } if @tool_choice == :none + end params end diff --git a/app/models/provider/llm_concept.rb b/app/models/provider/llm_concept.rb index c4ee70ef7..698bf13ee 100644 --- a/app/models/provider/llm_concept.rb +++ b/app/models/provider/llm_concept.rb @@ -40,6 +40,7 @@ module Provider::LlmConcept instructions: nil, functions: [], function_results: [], + tool_choice: nil, messages: nil, conversation_history: [], streamer: nil, diff --git a/app/models/provider/openai.rb b/app/models/provider/openai.rb index c28114273..1c6f5d86b 100644 --- a/app/models/provider/openai.rb +++ b/app/models/provider/openai.rb @@ -73,21 +73,47 @@ class Provider::Openai < Provider # out of the box. Users on larger-context cloud models can raise via ENV or # via the Self-Hosting settings page. def context_window - positive_budget(ENV["LLM_CONTEXT_WINDOW"], Setting.llm_context_window, 2048) + # Single source of truth shared with prompt assembly, so the assistant's + # collapse-to-counts decision always agrees with the window used here. + Assistant::TokenBudget.context_window end def max_response_tokens positive_budget(ENV["LLM_MAX_RESPONSE_TOKENS"], Setting.llm_max_response_tokens, 512) end + # The response cap is only sent to the provider when someone explicitly + # configured it (ENV or a stored Setting). The 512 fallback above exists for + # budget math and must not silently truncate replies on stock installs. + def explicit_max_response_tokens + explicit = ENV["LLM_MAX_RESPONSE_TOKENS"].to_s.strip.to_i + return explicit if explicit.positive? + + from_setting = Setting.llm_max_response_tokens.to_i + return from_setting if from_setting.positive? + + nil + end + def system_prompt_reserve positive_budget(ENV["LLM_SYSTEM_PROMPT_RESERVE"], nil, 256) end - def max_history_tokens + def max_history_tokens(instructions: nil) explicit = ENV["LLM_MAX_HISTORY_TOKENS"].presence&.to_i return explicit if explicit&.positive? - [ context_window - max_response_tokens - system_prompt_reserve, 256 ].max + + # When the actual instructions are in hand, budget against their real + # estimated size instead of the flat reserve; the prompt with session + # context routinely exceeds the historical 256-token figure. + prompt_reserve = + if instructions.present? + Assistant::TokenEstimator.estimate(instructions.to_s) + else + system_prompt_reserve + end + + [ context_window - max_response_tokens - prompt_reserve, 256 ].max end # Budget available for a one-shot (non-chat) request's full input, @@ -263,6 +289,7 @@ class Provider::Openai < Provider instructions: nil, functions: [], function_results: [], + tool_choice: nil, messages: nil, conversation_history: [], streamer: nil, @@ -281,6 +308,7 @@ class Provider::Openai < Provider instructions: instructions, functions: functions, function_results: function_results, + tool_choice: tool_choice, streamer: streamer, previous_response_id: previous_response_id, session_id: session_id, @@ -294,6 +322,7 @@ class Provider::Openai < Provider instructions: instructions, functions: functions, function_results: function_results, + tool_choice: tool_choice, messages: messages, streamer: streamer, session_id: session_id, @@ -335,6 +364,7 @@ class Provider::Openai < Provider instructions: nil, functions: [], function_results: [], + tool_choice: nil, streamer: nil, previous_response_id: nil, session_id: nil, @@ -366,14 +396,18 @@ class Provider::Openai < Provider input_payload = chat_config.build_input(prompt: prompt) begin - raw_response = client.responses.create(parameters: { + request_params = { model: model, input: input_payload, instructions: instructions, tools: chat_config.tools, previous_response_id: previous_response_id, stream: stream_proxy - }) + } + request_params[:tool_choice] = "none" if tool_choice == :none && chat_config.tools.present? + request_params[:max_output_tokens] = explicit_max_response_tokens if explicit_max_response_tokens + + raw_response = client.responses.create(parameters: request_params) # If streaming, Ruby OpenAI does not return anything, so to normalize this method's API, we search # for the "response chunk" in the stream and return it (it is already parsed) @@ -438,6 +472,7 @@ class Provider::Openai < Provider instructions: nil, functions: [], function_results: [], + tool_choice: nil, messages: nil, streamer: nil, session_id: nil, @@ -460,6 +495,8 @@ class Provider::Openai < Provider messages: messages } params[:tools] = tools if tools.present? + params[:tool_choice] = "none" if tool_choice == :none && tools.present? + params[:max_tokens] = explicit_max_response_tokens if explicit_max_response_tokens begin raw_response = client.chat(parameters: params) @@ -521,7 +558,7 @@ class Provider::Openai < Provider # LocalAI) don't silently truncate. tool_call/tool_result pairs are # preserved atomically by HistoryTrimmer. if messages.present? - trimmed = Assistant::HistoryTrimmer.new(messages, max_tokens: max_history_tokens).call + trimmed = Assistant::HistoryTrimmer.new(messages, max_tokens: max_history_tokens(instructions: instructions)).call payload.concat(trimmed) elsif prompt.present? payload << { role: "user", content: prompt } diff --git a/db/eval_data/chat_golden_v2.yml b/db/eval_data/chat_golden_v2.yml new file mode 100644 index 000000000..09f088009 --- /dev/null +++ b/db/eval_data/chat_golden_v2.yml @@ -0,0 +1,132 @@ +--- +name: chat_golden_v2 +description: > + Golden dataset exercising the production system prompt and full tool + registry: request classification (CHAT questions must use no tools), + specific-tool preference (aggregates via statements, not transaction + paging), and the newer analytical tools. Note the harness is single-shot; + it observes the first round's function selection and text, it does not + execute tools, so assertions here are about routing and phrasing rather + than final numbers. +eval_type: chat +version: "1.0" +metadata: + created_at: "2026-08-16" + source: manual_curation + +samples: + # ===== Classification: CHAT means no tools ===== + - id: chat_v2_class_001 + difficulty: easy + tags: [classification, no_tools] + input: + prompt: "Thanks, that was helpful!" + expected: + functions: [] + response_contains: [] + + - id: chat_v2_class_002 + difficulty: easy + tags: [classification, no_tools] + input: + prompt: "What does APR mean on a credit card?" + expected: + functions: [] + response_contains: ["annual"] + + - id: chat_v2_class_003 + difficulty: medium + tags: [classification, no_tools] + input: + prompt: "In general, how does compound interest work?" + expected: + functions: [] + response_contains: ["interest"] + + # ===== Specific-tool preference ===== + - id: chat_v2_route_001 + difficulty: medium + tags: [get_income_statement, tool_preference] + input: + prompt: "How much did I spend this year?" + expected: + functions: + - name: "get_income_statement" + response_contains: [] + + - id: chat_v2_route_002 + difficulty: medium + tags: [get_balance_sheet, tool_preference] + input: + prompt: "How has my net worth changed over the last year?" + expected: + functions: + - name: "get_balance_sheet" + response_contains: [] + + - id: chat_v2_route_003 + difficulty: easy + tags: [get_transactions, tool_preference] + input: + prompt: "Find my transactions at Costco last month" + expected: + functions: + - name: "get_transactions" + response_contains: [] + + # ===== New analytical tools ===== + - id: chat_v2_new_001 + difficulty: easy + tags: [get_recurring_transactions] + input: + prompt: "What subscriptions am I paying for?" + expected: + functions: + - name: "get_recurring_transactions" + response_contains: [] + + - id: chat_v2_new_002 + difficulty: medium + tags: [get_recurring_transactions] + input: + prompt: "Which bills are coming up in the next two weeks?" + expected: + functions: + - name: "get_recurring_transactions" + params: + upcoming_within_days: 14 + response_contains: [] + + - id: chat_v2_new_003 + difficulty: easy + tags: [get_merchants] + input: + prompt: "Which merchants do I shop at?" + expected: + functions: + - name: "get_merchants" + response_contains: [] + + - id: chat_v2_new_004 + difficulty: medium + tags: [get_income_statement, trends] + input: + prompt: "Show my spending month by month for this year" + expected: + functions: + - name: "get_income_statement" + params: + group_by: "month" + response_contains: [] + + - id: chat_v2_new_005 + difficulty: medium + tags: [get_transactions, sorting] + input: + prompt: "What were my five largest expenses this month?" + expected: + functions: + - name: "get_transactions" + params: + sort_by: "amount" + response_contains: [] diff --git a/docs/hosting/ai.md b/docs/hosting/ai.md index 9540900ef..39f446460 100644 --- a/docs/hosting/ai.md +++ b/docs/hosting/ai.md @@ -424,29 +424,9 @@ For OAuth clients, `` is the issued Doorkeeper bearer token. For s | `tools/list` | Lists available tools with names, descriptions, and input schemas | | `tools/call` | Calls a specific tool by name with arguments | -**Base tools** (exposed via `tools/list`; treat the live response as the source of truth): - -| Tool | Description | -|------|-------------| -| `get_accounts` | Retrieve account information | -| `get_transactions` | Query transaction history | -| `get_holdings` | Investment holdings data | -| `get_balance_sheet` | Current financial position | -| `get_income_statement` | Income and expenses | -| `get_budget` | Budget status and category breakdowns | -| `import_bank_statement` | Import bank statement data | -| `search_family_files` | Search documents uploaded through the import flow | -| `create_goal` | Create a savings goal | -| `get_tags` | List family tags | -| `create_tag` | Create a family tag | -| `update_tag` | Update a family tag | -| `get_categories` | List family categories | -| `create_category` | Create a family category | -| `update_category` | Update a family category | -| `update_transaction` | Update an existing transaction | -| `update_budget` | Update a budget category allocation | - -Preview users may also see Statement Vault tools in `tools/list`. +**Available tools** (exposed via `tools/list`): the `/mcp` endpoint serves the +same registry as the builtin assistant. See the canonical tool tables in +[mcp.md](mcp.md#available-tools), which also cover the preview tools. **Example: list tools** ```bash @@ -708,29 +688,43 @@ Assistant.for_chat(chat) # => Assistant::Builtin instance ### Function Registry -The `Assistant.function_classes` method centralizes all available financial tools: - -```ruby -def self.function_classes - [ - Function::GetTransactions, - Function::GetAccounts, - Function::GetHoldings, - Function::GetBalanceSheet, - Function::GetIncomeStatement, - Function::GetBudget, - Function::ImportBankStatement, - Function::SearchFamilyFiles, - Function::CreateGoal - ] -end -``` +`Assistant.function_classes(user = nil)` centralizes all available financial +tools. The full list lives in `app/models/assistant.rb` (not repeated here; +it drifts). Passing a user matters: preview tools +(`PREVIEW_FUNCTION_CLASSES`) are appended only when that user has preview +features enabled. These functions are: - Used by builtin assistants for LLM function calling - Exposed via the MCP endpoint for external agents - Defined in `app/models/assistant/function/` +### Responder loop + +`Assistant::Responder` drives the tool loop. Facts that matter when adding a +function: + +- The iteration cap counts **rounds** (model round-trips), not individual + calls; parallel calls in one round count once. Default 8, override with + `ASSISTANT_MAX_TOOL_CALL_ITERATIONS`. +- On the final permitted round the follow-up request offers **no tools**, so + the model must answer in text with whatever it gathered (a grace turn + instead of a dead chat). `ToolCallLimitError` remains as a backstop. +- Tool failures do not raise out of the loop. `FunctionToolCaller` returns + `{error:, hint:}` results, and the system prompt tells the model to follow + the hint and retry exactly once. Functions should return + `{ error: "...", message: "..." }`-shaped soft failures for expected + problems (unknown ids, invalid dates) rather than raising. + +### System prompt structure + +The prompt is `Assistant::Configurable::STATIC_INSTRUCTIONS` (a frozen, +byte-stable constant, which providers can cache as a repeated prefix) +followed by a volatile `## Session context` block: date, formats, currency, +an account roster and category names. The roster collapses to counts beyond +25 accounts, categories beyond 60 names, and both collapse whenever the +configured context window is below 4096 tokens. + ### Adding a New Assistant Type To add a custom assistant implementation: @@ -1139,10 +1133,10 @@ Then restart both `web` and `worker` so the new env var is loaded. If you are us **Cause:** Three settings interact here, measured over different spans: - `OPENAI_REQUEST_TIMEOUT` (default `60`) — applies to **each HTTP call** to the model, on its own. -- `ASSISTANT_MAX_TOOL_CALL_ITERATIONS` (default `5`) — how many chained tool calls one turn may make. A turn costs up to `1 + this` model calls. +- `ASSISTANT_MAX_TOOL_CALL_ITERATIONS` (default `8`) — how many chained tool-call rounds one turn may make. A turn costs up to `1 + this` model calls. On the final permitted round the model is offered no tools, so it answers in text instead of erroring. - `AI_RESPONSE_TIMEOUT` (default `90`) — covers the **whole turn**, and its clock starts when the message is queued, so Sidekiq queue time counts against it. -Responses from custom OpenAI-compatible providers are **not streamed**, so nothing appears in the chat until the entire reply is generated. Worse, the assistant only shows text once a response actually contains some — a tool-call-only response produces nothing to display — so a turn that chains several tool calls sits on "Thinking…" through all of them. At the defaults the worst case is six sequential model calls plus five tool executions. +Responses from custom OpenAI-compatible providers are **not streamed**, so nothing appears in the chat until the entire reply is generated. Worse, the assistant only shows text once a response actually contains some — a tool-call-only response produces nothing to display — so a turn that chains several tool calls sits on "Thinking…" through all of them. At the defaults the worst case is nine sequential model calls plus eight tool-round executions. **Fix:** size `AI_RESPONSE_TIMEOUT` as a **sum**, not simply as a number larger than the per-call limit: @@ -1151,7 +1145,7 @@ AI_RESPONSE_TIMEOUT ≥ (1 + ASSISTANT_MAX_TOOL_CALL_ITERATIONS) × OPENAI_REQUE + tool execution + queue wait ``` -You have two levers, and the cheaper one is usually the tool-call cap, because it divides the first term. With `ASSISTANT_MAX_TOOL_CALL_ITERATIONS=2` a turn costs at most three model calls instead of six, halving the timeout you need. The trade-off is that genuinely long tool chains fail earlier, with a clear "exceeded the tool-call limit" error rather than a timeout. +You have two levers, and the cheaper one is usually the tool-call cap, because it divides the first term. With `ASSISTANT_MAX_TOOL_CALL_ITERATIONS=2` a turn costs at most three model calls instead of nine, cutting the timeout you need by two-thirds. The trade-off is that genuinely long tool chains fail earlier, with a clear "exceeded the tool-call limit" error rather than a timeout. ```bash OPENAI_REQUEST_TIMEOUT=300 @@ -1159,7 +1153,7 @@ ASSISTANT_MAX_TOOL_CALL_ITERATIONS=2 AI_RESPONSE_TIMEOUT=1200 # (1 + 2) × 300 = 900, plus 300 headroom ``` -Keeping the full five iterations at 300s per call would instead need `6 × 300 = 1800` plus headroom — which is why lowering the cap is usually the better trade. +Keeping the full eight iterations at 300s per call would instead need `9 × 300 = 2700` plus headroom — which is why lowering the cap is usually the better trade. If `AI_RESPONSE_TIMEOUT` ends up below what the turn actually takes, you get a generic "no response" instead of the specific timeout error, and the job keeps running and burning tokens after the chat has given up. @@ -1234,9 +1228,16 @@ Restart `web` and `worker` after changing the environment variables, and make su The builtin AI assistant uses a system prompt that defines its behavior. The prompt is defined in `app/models/assistant/configurable.rb`. This does not apply to external assistants, which manage their own prompts. +The prompt has two halves: `STATIC_INSTRUCTIONS`, a frozen constant that is +byte-identical on every request (providers cache and discount an +exactly-repeated prefix), and a trailing `## Session context` block holding +everything volatile (date, currency, account roster, categories). + To customize: 1. Fork the repository -2. Edit the `default_instructions` method +2. Edit the `STATIC_INSTRUCTIONS` constant (keep customizations there so the + prompt stays cacheable; only put genuinely per-request data in the session + context builders) 3. Rebuild and deploy **What you can customize:** @@ -1250,17 +1251,29 @@ To customize: The assistant uses OpenAI's function calling (tool use) to access user data: **Available functions:** -- `get_transactions` - Retrieve transaction history -- `get_accounts` - Get account information -- `get_holdings` - Investment holdings data -- `get_balance_sheet` - Current financial position -- `get_income_statement` - Income and expenses -- `get_budget` - Budget status and category breakdowns -- `import_bank_statement` - Import bank statement data -- `search_family_files` - Search uploaded documents -- `create_goal` - Create a savings goal -These are defined in `app/models/assistant/function/`. +Read and analysis: +- `get_transactions` - Search transactions with filters, sorting and pagination +- `get_recurring_transactions` - Detected and manual recurring transactions (subscriptions, bills) with totals +- `get_accounts` - Accounts with ids and current balances; opt-in balance history series +- `get_holdings` - Investment holdings +- `get_balance_sheet` - Net worth, assets and liabilities with a configurable history period and interval +- `get_income_statement` - Income and expenses for a period, with monthly series, prior-period comparison and account filtering +- `get_budget` - Budget summary for a month +- `get_merchants` - Merchants with the ids update_transaction accepts and the exact names get_transactions filters on +- `get_tags` / `get_categories` - Tag and category listings with pagination + +Write: +- `update_transaction`, `update_budget`, `create_goal` +- `create_tag` / `update_tag`, `create_category` / `update_category` + +Documents: +- `import_bank_statement` - Import bank statement data +- `search_family_files` - Search uploaded documents (vector store) + +These are defined in `app/models/assistant/function/`. Preview tools +(Statement Vault, `get_valuations`, `get_insights`) are listed in +[mcp.md](mcp.md#preview-tools). ### Vector Store (Document Search) diff --git a/docs/hosting/mcp.md b/docs/hosting/mcp.md index 8632fc734..e22598dce 100644 --- a/docs/hosting/mcp.md +++ b/docs/hosting/mcp.md @@ -135,23 +135,23 @@ At the time of writing, `tools/list` includes: | Tool | Description | |------|-------------| -| `get_transactions` | Retrieve transaction history with filtering | -| `get_accounts` | Get account information and balances | +| `get_transactions` | Search transactions with filters (exact names or ids), sorting by date or absolute amount, and pagination | +| `get_recurring_transactions` | Detected and manual recurring transactions (subscriptions, bills, salaries) with expected dates and per-currency totals | +| `get_accounts` | Accounts with ids and current balances; pass `include_balance_series: true` for a period-bounded history series | | `get_holdings` | Query investment holdings | -| `get_balance_sheet` | Current financial position (assets, liabilities, net worth) | -| `get_income_statement` | Income and expenses over a period | -| `get_budget` | Budget status and budget category breakdowns | +| `get_balance_sheet` | Net worth, assets and liabilities with a configurable history period and interval | +| `get_income_statement` | Income and expenses for a period, with optional monthly series, prior-period comparison and account filtering | +| `get_budget` | Budget summary for a month, with optional prior months | +| `get_merchants` | Merchants with the ids `update_transaction` accepts and the exact names `get_transactions` filters on | +| `get_tags` | Tags with pagination | +| `get_categories` | Categories with hierarchy and pagination | +| `create_goal` | Create a savings goal linked to depository accounts | +| `create_tag` / `update_tag` | Manage tags | +| `create_category` / `update_category` | Manage categories | +| `update_transaction` | Edit a transaction's metadata (name, notes, category, merchant, tags) | +| `update_budget` | Update budget allocations for a month | | `import_bank_statement` | Import bank statement data | | `search_family_files` | Search documents uploaded through the import flow. Note this is the vector-store document index, not the Statement Vault — statements archived via `upload_account_statement` are not searchable through it | -| `create_goal` | Create a savings goal linked to depository accounts | -| `get_tags` | List family tags | -| `create_tag` | Create a family tag | -| `update_tag` | Update a family tag | -| `get_categories` | List family categories | -| `create_category` | Create a family category | -| `update_category` | Update a family category | -| `update_transaction` | Update an existing transaction | -| `update_budget` | Update a budget category allocation | These are the same tools used by Sure's built-in AI assistant. @@ -170,6 +170,8 @@ permissions enforced in the web UI. | `get_account_statement` | One statement's details and its reconciliation checks against the ledger — present only once someone has entered the statement's opening/closing balances in the web UI, since nothing extracts them from the document. Does not return the file: stored documents are served only to a signed-in browser session | | `get_statement_coverage` | Month-by-month statement coverage for an account: `covered`, `missing`, `mismatched`, `ambiguous`, `duplicate`, `not_expected`, each with a reconciliation status | | `record_valuation` | Record an account's value on a date, with a required source citation | +| `get_valuations` | List recorded valuations newest first, including the citation stored in each entry's notes; the read pair for `record_valuation` | +| `get_insights` | Read the proactive insights feed (spending anomalies, cash-flow warnings, subscription audits and more) without marking anything read | They exist for agents that maintain a document-backed record of a family's wealth over time. See diff --git a/test/models/assistant/configurable_test.rb b/test/models/assistant/configurable_test.rb index 0dc65c728..c686e9604 100644 --- a/test/models/assistant/configurable_test.rb +++ b/test/models/assistant/configurable_test.rb @@ -18,4 +18,64 @@ class AssistantConfigurableTest < ActiveSupport::TestCase assert_equal [], config[:functions] assert_includes config[:instructions], "stage of life" end + + test "instructions start with the byte-stable static block and end with session context" do + chat = chats(:one) + + instructions = Assistant.config_for(chat)[:instructions] + + assert instructions.start_with?(Assistant::Configurable::STATIC_INSTRUCTIONS) + assert_operator instructions.index("## Session context"), :>, instructions.index("### Rules about financial advice") + assert_includes instructions, "Today's date: #{Date.current}" + end + + test "session context lists accounts and categories for a typical family" do + chat = chats(:one) + family = chat.user.family + + # The roster only itemizes when the model's context window is comfortable + Setting.stubs(:llm_context_window).returns(128_000) + + instructions = Assistant.config_for(chat)[:instructions] + + visible_account = chat.user.accessible_accounts.visible.first + + assert_includes instructions, "### Accounts" + assert_includes instructions, visible_account.name + assert_includes instructions, "### Categories" + assert_includes instructions, family.categories.first.name + assert_includes instructions, "Uncategorized" + end + + test "session context collapses to counts for large account rosters" do + chat = chats(:one) + user = chat.user + + Setting.stubs(:llm_context_window).returns(128_000) + + 26.times do |i| + user.family.accounts.create!( + name: "Roster Account #{i}", + balance: 100, + currency: "USD", + accountable: Depository.new + ) + end + + instructions = Assistant.config_for(chat)[:instructions] + + assert_match(/\d+ accounts:/, instructions) + assert_not_includes instructions, "Roster Account 1:" + end + + test "session context collapses to counts on small context windows" do + chat = chats(:one) + + Setting.stubs(:llm_context_window).returns(2048) + + instructions = Assistant.config_for(chat)[:instructions] + + assert_match(/\d+ accounts:/, instructions) + assert_match(/\d+ categories\./, instructions) + end end diff --git a/test/models/assistant/function/get_accounts_test.rb b/test/models/assistant/function/get_accounts_test.rb new file mode 100644 index 000000000..40fb6c69e --- /dev/null +++ b/test/models/assistant/function/get_accounts_test.rb @@ -0,0 +1,87 @@ +require "test_helper" + +class Assistant::Function::GetAccountsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @fn = Assistant::Function::GetAccounts.new(@user) + end + + test "has correct name" do + assert_equal "get_accounts", @fn.name + end + + test "has a description" do + assert_not_empty @fn.description + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "returns account ids and omits the balance series by default" do + result = @fn.call + + assert result[:accounts].any? + + result[:accounts].each do |account| + assert account[:id].present? + assert_not account.key?(:historical_balances) + end + end + + test "excludes hidden accounts" do + hidden = @family.accounts.visible.first + hidden.update!(status: "disabled") + + result = @fn.call + + assert_not_includes result[:accounts].map { |a| a[:id] }, hidden.id + end + + test "includes a balance series bounded by the requested period when asked" do + result = @fn.call({ "include_balance_series" => true, "series_period" => "last_30_days" }) + + account = result[:accounts].first + series = account[:historical_balances] + + assert series.present? + assert series[:start_date] >= 30.days.ago.to_date + assert_equal Date.current, series[:end_date] + assert(series[:values].all? { |v| v.is_a?(Numeric) }) + end + + test "falls back to last_365_days for an unknown series period" do + result = @fn.call({ "include_balance_series" => true, "series_period" => "bogus" }) + + series = result[:accounts].first[:historical_balances] + + assert series[:start_date] >= 366.days.ago.to_date + end + + test "an account starting beyond the period skips its series without failing the call" do + future_account = @family.accounts.create!( + name: "Future Start Account", + balance: 0, + currency: "USD", + accountable: Depository.new + ) + future_account.entries.create!( + name: "Scheduled opening deposit", + date: 30.days.from_now.to_date, + amount: -100, + currency: "USD", + entryable: Transaction.new + ) + + result = @fn.call({ "include_balance_series" => true, "series_period" => "last_7_days" }) + + assert_not result.key?(:error) + + future_payload = result[:accounts].find { |a| a[:id] == future_account.id } + + assert_not_nil future_payload + assert_not future_payload.key?(:historical_balances) + assert(result[:accounts].any? { |a| a.key?(:historical_balances) }) + end +end diff --git a/test/models/assistant/function/get_balance_sheet_test.rb b/test/models/assistant/function/get_balance_sheet_test.rb new file mode 100644 index 000000000..dd5e65edc --- /dev/null +++ b/test/models/assistant/function/get_balance_sheet_test.rb @@ -0,0 +1,51 @@ +require "test_helper" + +class Assistant::Function::GetBalanceSheetTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @fn = Assistant::Function::GetBalanceSheet.new(@user) + end + + test "has correct name" do + assert_equal "get_balance_sheet", @fn.name + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "default call preserves the established shape" do + result = @fn.call + + assert result[:net_worth][:current].present? + assert result[:assets].key?(:monthly_history) + assert result[:liabilities].key?(:monthly_history) + assert result[:insights][:debt_to_asset_ratio].present? + end + + test "named period bounds the history series" do + result = @fn.call("period" => "last_30_days") + + series = result[:net_worth][:monthly_history] + + assert series[:start_date] >= 31.days.ago.to_date + end + + test "invalid custom dates return an invalid_date error" do + result = @fn.call("start_date" => "not-a-date", "end_date" => "2024-01-01") + + assert_equal "invalid_date", result[:error] + end + + test "a reversed custom range returns the structured error" do + result = @fn.call("start_date" => "2025-06-01", "end_date" => "2025-01-01") + + assert_equal "invalid_date", result[:error] + end + + test "too many points returns an error instead of a giant series" do + result = @fn.call("period" => "last_10_years", "interval" => "1 day") + + assert_equal "too_many_points", result[:error] + end +end diff --git a/test/models/assistant/function/get_categories_test.rb b/test/models/assistant/function/get_categories_test.rb index 8497ba3cb..30d2fa342 100644 --- a/test/models/assistant/function/get_categories_test.rb +++ b/test/models/assistant/function/get_categories_test.rb @@ -83,4 +83,20 @@ class Assistant::Function::GetCategoriesTest < ActiveSupport::TestCase assert_equal 2, page2[:page] assert_not_equal page1[:categories].map { |c| c[:name] }, page2[:categories].map { |c| c[:name] } end + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "honors and clamps page_size" do + 10.times { |i| @family.categories.create!(name: "SizedCategory#{format('%02d', i)}", color: "#e99537", lucide_icon: "shapes") } + + result = @fn.call({ "page_size" => 5 }) + + assert_equal 5, result[:page_size] + assert_equal 5, result[:categories].size + + clamped = @fn.call({ "page_size" => 5000 }) + + assert_equal 100, clamped[:page_size] + end end diff --git a/test/models/assistant/function/get_income_statement_test.rb b/test/models/assistant/function/get_income_statement_test.rb new file mode 100644 index 000000000..468641464 --- /dev/null +++ b/test/models/assistant/function/get_income_statement_test.rb @@ -0,0 +1,184 @@ +require "test_helper" + +class Assistant::Function::GetIncomeStatementTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @fn = Assistant::Function::GetIncomeStatement.new(@user) + @params = { + "start_date" => 1.year.ago.to_date.to_s, + "end_date" => Date.current.to_s + } + end + + test "has correct name" do + assert_equal "get_income_statement", @fn.name + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "happy path shape is unchanged" do + result = @fn.call(@params) + + assert result[:income][:total].present? + assert result[:expense][:total].present? + assert result[:income].key?(:by_category) + assert result[:insights][:net_income].present? + assert_not result.key?(:monthly_series) + assert_not result.key?(:previous_period) + end + + test "group_by month returns one bucket per calendar month" do + result = @fn.call(@params.merge( + "start_date" => "2025-01-15", + "end_date" => "2025-03-10", + "group_by" => "month" + )) + + series = result[:monthly_series] + + assert_equal 3, series.size + assert_equal Date.parse("2025-01-15"), series.first[:start_date] + assert_equal Date.parse("2025-01-31"), series.first[:end_date] + assert_equal Date.parse("2025-03-10"), series.last[:end_date] + series.each do |bucket| + assert bucket[:income].present? + assert bucket[:expenses].present? + assert bucket[:net].present? + end + end + + test "group_by month caps the bucket count" do + result = @fn.call( + "start_date" => "2020-01-01", + "end_date" => "2025-12-31", + "group_by" => "month" + ) + + assert_equal "too_many_periods", result[:error] + end + + test "compare_previous_period returns an equal-length prior window with deltas" do + result = @fn.call(@params.merge( + "start_date" => "2025-06-01", + "end_date" => "2025-06-30", + "compare_previous_period" => true + )) + + previous = result[:previous_period] + + assert_equal Date.parse("2025-05-02"), previous[:start_date] + assert_equal Date.parse("2025-05-31"), previous[:end_date] + assert previous[:income_change].key?(:amount) + assert previous[:expenses_change].key?(:percent) + end + + test "account_ids scopes totals and omits the category breakdown" do + account = @family.accounts.visible.first + + result = @fn.call(@params.merge("account_ids" => [ account.id ])) + + assert_nil result[:income][:by_category] + assert result[:breakdown_omitted_reason].present? + assert result[:net].present? + end + + test "unknown account ids return a soft failure naming them" do + bogus = SecureRandom.uuid + + result = @fn.call(@params.merge("account_ids" => [ bogus ])) + + assert_equal "unknown_account_ids", result[:error] + assert_equal [ bogus ], result[:unknown_ids] + end + + test "inaccessible account ids are treated as unknown" do + result = Assistant::Function::GetIncomeStatement.new(users(:family_member)).call( + @params.merge("account_ids" => [ accounts(:investment).id ]) + ) + + assert_equal "unknown_account_ids", result[:error] + end + + test "accounts excluded from reports are rejected instead of returning silent zeros" do + excluded = @family.accounts.visible.first + excluded.update!(exclude_from_reports: true) + + result = @fn.call(@params.merge("account_ids" => [ excluded.id ])) + + assert_equal "unknown_account_ids", result[:error] + assert_includes result[:unknown_ids], excluded.id + end + + test "invalid dates return an invalid_date error" do + result = @fn.call("start_date" => "bogus", "end_date" => "2024-01-01") + + assert_equal "invalid_date", result[:error] + end + + # The assistant and MCP paths run without a session, so Current.user is nil + # and an unscoped IncomeStatement reports family-wide totals. Every read in + # this tool must resolve through the user-scoped statement, or the numbers it + # reports disagree with the account ids it validates against. + test "totals stay scoped to the requesting user when there is no session" do + Current.session = nil + other = users(:family_member) + + foreign = Account.create!( + family: @family, + name: "Another members checking", + currency: "USD", + balance: 0, + owner: other, + accountable: Depository.new + ) + foreign.entries.create!( + name: "Spend outside this users finances", + date: Date.current, + amount: 1234.56, + currency: "USD", + entryable: Transaction.new(category: categories(:food_and_drink)) + ) + @family.reload + + assert_not_includes @user.finance_accounts.pluck(:id), foreign.id, + "precondition: the foreign account must sit outside the requesting users finances" + + period = Period.custom( + start_date: Date.parse(@params["start_date"]), + end_date: Date.parse(@params["end_date"]) + ) + scoped = @family.income_statement(user: @user).expense_totals(period: period).total + unscoped = @family.income_statement(user: nil).expense_totals(period: period).total + + assert_operator unscoped, :>, scoped, + "precondition: the unscoped statement must actually differ, or this test proves nothing" + + result = @fn.call(@params) + + assert_equal scoped.to_f.round(2), + result[:expense][:total].gsub(/[^\d.-]/, "").to_f.round(2) + end + + test "eligible account validation and reported totals agree on scope" do + Current.session = nil + other = users(:family_member) + + foreign = Account.create!( + family: @family, + name: "Another members savings", + currency: "USD", + balance: 0, + owner: other, + accountable: Depository.new + ) + + # Rejected as ineligible, so its money must not reach the totals either. + result = @fn.call(@params.merge("account_ids" => [ foreign.id ])) + + assert_equal "unknown_account_ids", result[:error] + assert_includes result[:unknown_ids], foreign.id + end +end diff --git a/test/models/assistant/function/get_insights_test.rb b/test/models/assistant/function/get_insights_test.rb new file mode 100644 index 000000000..c5aee3f6f --- /dev/null +++ b/test/models/assistant/function/get_insights_test.rb @@ -0,0 +1,81 @@ +require "test_helper" + +class Assistant::Function::GetInsightsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @fn = Assistant::Function::GetInsights.new(@user) + end + + test "has correct name" do + assert_equal "get_insights", @fn.name + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "returns visible insights with their metadata" do + result = @fn.call + + assert result[:insights].any? + + anomaly = result[:insights].find { |i| i[:type] == "spending_anomaly" } + + assert_not_nil anomaly + assert anomaly[:title].present? + assert anomaly[:metadata].present? + end + + test "excludes acknowledged insights by default and includes them on request" do + acknowledged = @family.insights.first + acknowledged.update!(status: "acknowledged") + + default_ids = @fn.call[:insights].map { |i| i[:id] } + + assert_not_includes default_ids, acknowledged.id + + included_ids = @fn.call("include_acknowledged" => true)[:insights].map { |i| i[:id] } + + assert_includes included_ids, acknowledged.id + end + + test "filters by insight_type and clamps limit" do + result = @fn.call("insight_type" => "cash_flow_warning", "limit" => 999) + + assert result[:insights].all? { |i| i[:type] == "cash_flow_warning" } + end + + test "limit clamps to the maximum with more matching insights than the cap" do + (Assistant::Function::GetInsights::MAX_LIMIT + 5).times do |i| + @family.insights.create!( + insight_type: "spending_anomaly", + priority: "low", + status: "active", + title: "Clamp insight #{i}", + body: "Body #{i}", + currency: "USD", + period_start: Date.current.beginning_of_month, + period_end: Date.current.end_of_month, + generated_at: Time.current, + dedup_key: "clamp-test-#{i}" + ) + end + + result = @fn.call("limit" => 999) + + assert_equal Assistant::Function::GetInsights::MAX_LIMIT, result[:insights].size + end + + test "does not mutate insight status" do + assert_no_changes -> { @family.insights.order(:id).pluck(:status) } do + @fn.call + end + end + + test "does not return another family's insights" do + result = Assistant::Function::GetInsights.new(users(:empty)).call + + assert_empty result[:insights] + end +end diff --git a/test/models/assistant/function/get_merchants_test.rb b/test/models/assistant/function/get_merchants_test.rb new file mode 100644 index 000000000..b618bbbfa --- /dev/null +++ b/test/models/assistant/function/get_merchants_test.rb @@ -0,0 +1,53 @@ +require "test_helper" + +class Assistant::Function::GetMerchantsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @fn = Assistant::Function::GetMerchants.new(@user) + end + + test "has correct name" do + assert_equal "get_merchants", @fn.name + end + + test "has a description" do + assert_not_empty @fn.description + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "returns family merchants with ids and source" do + result = @fn.call + + netflix = result[:merchants].find { |m| m[:name] == "Netflix" } + + assert_not_nil netflix + assert_equal merchants(:netflix).id, netflix[:id] + assert_equal "family", netflix[:source] + end + + test "filters by search substring case-insensitively" do + result = @fn.call("search" => "netfl") + + assert_equal [ "Netflix" ], result[:merchants].map { |m| m[:name] } + end + + test "honors and clamps page_size" do + result = @fn.call("page_size" => 1) + + assert_equal 1, result[:page_size] + assert_equal 1, result[:merchants].size + assert result[:total_pages] > 1 + end + + test "does not return another family's merchants" do + foreign_user = users(:empty) + + result = Assistant::Function::GetMerchants.new(foreign_user).call + + assert_empty result[:merchants].map { |m| m[:name] } & @family.merchants.pluck(:name) + end +end diff --git a/test/models/assistant/function/get_recurring_transactions_test.rb b/test/models/assistant/function/get_recurring_transactions_test.rb new file mode 100644 index 000000000..850245505 --- /dev/null +++ b/test/models/assistant/function/get_recurring_transactions_test.rb @@ -0,0 +1,79 @@ +require "test_helper" + +class Assistant::Function::GetRecurringTransactionsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @fn = Assistant::Function::GetRecurringTransactions.new(@user) + end + + test "has correct name" do + assert_equal "get_recurring_transactions", @fn.name + end + + test "has a description" do + assert_not_empty @fn.description + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "defaults to active recurring transactions only" do + result = @fn.call + + names = result[:recurring_transactions].map { |rt| rt[:name] } + + assert_includes names, "Netflix" + assert_not_includes names, "Amazon" + assert result[:recurring_transactions].all? { |rt| rt[:status] == "active" } + end + + test "status all returns inactive items too" do + result = @fn.call("status" => "all") + + assert_includes result[:recurring_transactions].map { |rt| rt[:name] }, "Amazon" + end + + test "upcoming_within_days windows on next expected date" do + result = @fn.call("upcoming_within_days" => 2) + + assert_empty result[:recurring_transactions] + + wide = @fn.call("upcoming_within_days" => 30) + + assert_includes wide[:recurring_transactions].map { |rt| rt[:name] }, "Netflix" + end + + test "totals sum active items per currency and exclude transfers" do + checking = @family.accounts.visible.first + savings = @family.accounts.visible.second + + @family.recurring_transactions.create!( + name: "Vault transfer", + account: checking, + destination_account: savings, + amount: 500, + currency: "USD", + expected_day_of_month: 1, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: 1.month.from_now.to_date, + status: "active", + occurrence_count: 4 + ) + + result = @fn.call("status" => "all") + transfer_row = result[:recurring_transactions].find { |rt| rt[:name] == "Vault transfer" } + + assert transfer_row[:is_transfer] + assert_equal "$15.99", result[:totals_by_currency]["USD"] + assert_equal false, result[:truncated] + assert_equal result[:recurring_transactions].size, result[:total_results] + end + + test "does not return another family's recurring transactions" do + result = Assistant::Function::GetRecurringTransactions.new(users(:empty)).call("status" => "all") + + assert_empty result[:recurring_transactions] + end +end diff --git a/test/models/assistant/function/get_tags_test.rb b/test/models/assistant/function/get_tags_test.rb index c734fe90f..9a8cf4fe9 100644 --- a/test/models/assistant/function/get_tags_test.rb +++ b/test/models/assistant/function/get_tags_test.rb @@ -64,4 +64,20 @@ class Assistant::Function::GetTagsTest < ActiveSupport::TestCase assert_equal 2, page2[:page] assert_not_equal page1[:tags].map { |t| t[:name] }, page2[:tags].map { |t| t[:name] } end + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "honors and clamps page_size" do + 10.times { |i| @family.tags.create!(name: "SizedTag#{format('%02d', i)}") } + + result = @fn.call({ "page_size" => 5 }) + + assert_equal 5, result[:page_size] + assert_equal 5, result[:tags].size + + clamped = @fn.call({ "page_size" => 0 }) + + assert_equal 1, clamped[:page_size] + end end diff --git a/test/models/assistant/function/get_transactions_test.rb b/test/models/assistant/function/get_transactions_test.rb index 3e1fe83a4..9d5f14482 100644 --- a/test/models/assistant/function/get_transactions_test.rb +++ b/test/models/assistant/function/get_transactions_test.rb @@ -41,4 +41,52 @@ class Assistant::Function::GetTransactionsTest < ActiveSupport::TestCase assert_empty result[:transactions] end + + test "schema no longer inlines user data enums" do + schema = @function.params_schema + + %i[accounts categories merchants tags].each do |key| + items = schema[:properties][key][:items] + + assert_equal({ type: "string" }, items, "#{key} should be a plain string array") + end + end + + test "honors page_size" do + result = @function.call("page_size" => 1) + + assert_equal 1, result[:page_size] + assert_equal 1, result[:transactions].size + assert result[:total_pages] > 1 + end + + test "sorts by absolute amount" do + result = @function.call("sort_by" => "amount", "order" => "desc") + + amounts = result[:transactions].map { |t| t[:amount].abs } + + assert_equal amounts.sort.reverse, amounts + end + + test "filters by type" do + result = @function.call("types" => [ "income" ]) + + assert result[:transactions].any? + assert result[:transactions].all? { |t| t[:classification] == "income" } + end + + test "filters by account_ids and ignores inaccessible ids" do + accessible_account = @transaction.entry.account + + result = @function.call("account_ids" => [ accessible_account.id ]) + + assert result[:transactions].any? + assert result[:transactions].all? { |t| t[:account] == accessible_account.name } + + member_result = Assistant::Function::GetTransactions.new(users(:family_member)).call( + "account_ids" => [ accounts(:investment).id ] + ) + + assert_empty member_result[:transactions] + end end diff --git a/test/models/assistant/function/get_valuations_test.rb b/test/models/assistant/function/get_valuations_test.rb new file mode 100644 index 000000000..6a55c7963 --- /dev/null +++ b/test/models/assistant/function/get_valuations_test.rb @@ -0,0 +1,92 @@ +require "test_helper" + +class Assistant::Function::GetValuationsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @account = @family.accounts.visible.first + @fn = Assistant::Function::GetValuations.new(@user) + + @entry = @account.entries.create!( + name: "Manual valuation", + date: Date.current, + amount: 12_345, + currency: @account.currency, + notes: "statement 2026-07 (grade: A)", + entryable: Valuation.new(kind: "reconciliation") + ) + end + + test "has correct name" do + assert_equal "get_valuations", @fn.name + end + + test "is not in strict mode" do + refute @fn.to_definition[:strict] + end + + test "lists valuations with kind and provenance notes" do + result = @fn.call + + row = result[:valuations].find { |v| v[:entry_id] == @entry.id } + + assert_not_nil row + assert_equal "reconciliation", row[:kind] + assert_equal "statement 2026-07 (grade: A)", row[:notes] + assert_equal @account.id, row[:account][:id] + end + + test "filters by account_id and validates its format" do + result = @fn.call("account_id" => @account.id) + + assert result[:valuations].all? { |v| v[:account][:id] == @account.id } + + invalid = @fn.call("account_id" => "not-a-uuid") + + assert_equal "invalid_account_id", invalid[:error] + end + + test "filters by date range" do + result = @fn.call("start_date" => Date.current.to_s, "end_date" => Date.current.to_s) + + assert_includes result[:valuations].map { |v| v[:entry_id] }, @entry.id + + earlier = @fn.call("end_date" => 1.year.ago.to_date.to_s) + + assert_not_includes earlier[:valuations].map { |v| v[:entry_id] }, @entry.id + end + + test "malformed dates fail loudly instead of silently dropping the filter" do + result = @fn.call("start_date" => "not-a-date") + + assert_equal "invalid_date", result[:error] + end + + test "a reversed date range returns the structured error" do + result = @fn.call("start_date" => Date.current.to_s, "end_date" => 1.year.ago.to_date.to_s) + + assert_equal "invalid_date", result[:error] + end + + test "invalid page numbers normalize to the first page" do + result = @fn.call("page" => 0) + + assert_equal 1, result[:page] + assert_includes result[:valuations].map { |v| v[:entry_id] }, @entry.id + end + + test "excludes valuations on accounts the user cannot access" do + member_fn = Assistant::Function::GetValuations.new(users(:family_member)) + investment_entry = accounts(:investment).entries.create!( + name: "Hidden valuation", + date: Date.current, + amount: 999, + currency: "USD", + entryable: Valuation.new(kind: "reconciliation") + ) + + result = member_fn.call + + assert_not_includes result[:valuations].map { |v| v[:entry_id] }, investment_entry.id + end +end diff --git a/test/models/assistant/function/schema_strictness_test.rb b/test/models/assistant/function/schema_strictness_test.rb new file mode 100644 index 000000000..ca6edeb19 --- /dev/null +++ b/test/models/assistant/function/schema_strictness_test.rb @@ -0,0 +1,34 @@ +require "test_helper" + +class Assistant::Function::SchemaStrictnessTest < ActiveSupport::TestCase + # Strict function calling requires every declared property to appear in + # `required`. A strict tool with an optional property produces an invalid + # schema that strict providers reject wholesale, so this walks the entire + # registry (preview tools included) to keep the class of bug out for good. + test "strict functions declare every property as required" do + user = users(:family_admin) + user.update!(preferences: (user.preferences || {}).merge("preview_features_enabled" => true)) + + function_classes = Assistant.function_classes(user) + + # Every shipped tool declares optional properties and so opts out of strict + # mode; the walk below therefore asserts nothing today and exists to catch + # the first tool that keeps the default. Guard the registry itself so a + # future empty or broken lookup cannot make this test vacuous unnoticed. + assert_operator function_classes.size, :>, 15, + "the tool registry looks empty or truncated, so the strictness walk would prove nothing" + + function_classes.each do |fn_class| + fn = fn_class.new(user) + definition = fn.to_definition + next unless definition[:strict] + + schema = definition[:params_schema] + property_keys = schema[:properties].keys.map(&:to_s) + required_keys = Array(schema[:required]).map(&:to_s) + + assert_equal property_keys.sort, required_keys.sort, + "#{fn.name} is strict but properties #{property_keys - required_keys} are not required" + end + end +end diff --git a/test/models/assistant/function_test.rb b/test/models/assistant/function_test.rb index 78ddd59d2..a27b2400e 100644 --- a/test/models/assistant/function_test.rb +++ b/test/models/assistant/function_test.rb @@ -90,6 +90,17 @@ class Assistant::FunctionTest < ActiveSupport::TestCase end end + test "to_ai_time_series rounds to the currency's own precision" do + value = Struct.new(:trend).new(Struct.new(:current).new(Money.new(0.00000001, "BTC"))) + series = Struct.new(:start_date, :end_date, :interval, :values) + .new(Date.current - 1, Date.current, "1 day", [ value ]) + + result = EmptyEnumFunction.new(nil).send(:to_ai_time_series, series) + + assert_equal "BTC", result[:currency] + assert_equal 0.00000001, result[:values].first + end + private def assert_no_empty_enums(node, function_name, path = "params_schema") case node diff --git a/test/models/assistant/function_tool_caller_test.rb b/test/models/assistant/function_tool_caller_test.rb index 8203e0d37..98a411379 100644 --- a/test/models/assistant/function_tool_caller_test.rb +++ b/test/models/assistant/function_tool_caller_test.rb @@ -51,4 +51,87 @@ class Assistant::FunctionToolCallerTest < ActiveSupport::TestCase assert_equal({}, result.function_result) end + + test "unknown tool returns an error result instead of raising" do + request = FunctionRequest.new( + id: "call_4", call_id: "call_4", function_name: "does_not_exist", + function_args: "{}" + ) + + result = assert_nothing_raised do + @caller.fulfill_requests([ request ]).first + end + + assert_equal "Unknown tool: does_not_exist", result.function_result["error"] + assert_includes result.function_result["hint"], "provided list" + end + + test "invalid JSON arguments return an error result with a hint" do + request = FunctionRequest.new( + id: "call_5", call_id: "call_5", function_name: "echo", + function_args: "{not json" + ) + + result = @caller.fulfill_requests([ request ]).first + + assert_equal "Arguments were not valid JSON", result.function_result["error"] + assert_includes result.function_result["hint"], "echo" + end + + test "missing records return an error result steering a corrected retry" do + caller = Assistant::FunctionToolCaller.new([ NotFoundFunction.new(nil) ]) + request = FunctionRequest.new( + id: "call_6", call_id: "call_6", function_name: "not_found", + function_args: "{}" + ) + + result = caller.fulfill_requests([ request ]).first + + assert_includes result.function_result["hint"], "retry once" + end + + test "date and argument errors return an error result with a format hint" do + caller = Assistant::FunctionToolCaller.new([ BadDateFunction.new(nil) ]) + request = FunctionRequest.new( + id: "call_7", call_id: "call_7", function_name: "bad_date", + function_args: "{}" + ) + + result = caller.fulfill_requests([ request ]).first + + assert_includes result.function_result["hint"], "YYYY-MM-DD" + end + + test "unexpected failures return a do-not-retry error result" do + caller = Assistant::FunctionToolCaller.new([ ExplodingFunction.new(nil) ]) + request = FunctionRequest.new( + id: "call_8", call_id: "call_8", function_name: "exploding", + function_args: "{}" + ) + + result = assert_nothing_raised do + caller.fulfill_requests([ request ]).first + end + + assert_equal "exploding failed unexpectedly", result.function_result["error"] + assert_includes result.function_result["hint"], "Do not retry" + end + + class NotFoundFunction < Assistant::Function + def self.name = "not_found" + def self.description = "Always raises RecordNotFound" + def call(params = {}) = raise(ActiveRecord::RecordNotFound, "Couldn't find Account") + end + + class BadDateFunction < Assistant::Function + def self.name = "bad_date" + def self.description = "Always raises a date parse error" + def call(params = {}) = Date.parse("not-a-date") + end + + class ExplodingFunction < Assistant::Function + def self.name = "exploding" + def self.description = "Always raises an unexpected error" + def call(params = {}) = raise(NoMethodError, "boom") + end end diff --git a/test/models/assistant/history_trimmer_test.rb b/test/models/assistant/history_trimmer_test.rb index abd94324e..3e5d40c90 100644 --- a/test/models/assistant/history_trimmer_test.rb +++ b/test/models/assistant/history_trimmer_test.rb @@ -90,4 +90,15 @@ class Assistant::HistoryTrimmerTest < ActiveSupport::TestCase assert_equal messages, result end + + test "keeps the newest group even when it alone exceeds the budget" do + messages = [ + { role: "user", content: "old message" }, + { role: "user", content: "the current question " * 200 } + ] + + result = Assistant::HistoryTrimmer.new(messages, max_tokens: 50).call + + assert_equal [ messages.last ], result + end end diff --git a/test/models/assistant/responder_test.rb b/test/models/assistant/responder_test.rb new file mode 100644 index 000000000..372afddf0 --- /dev/null +++ b/test/models/assistant/responder_test.rb @@ -0,0 +1,81 @@ +require "test_helper" + +class Assistant::ResponderTest < ActiveSupport::TestCase + include ProviderTestHelper + + class EchoFunction < Assistant::Function + def self.name = "echo" + def self.description = "Echoes the received arguments" + def call(params = {}) = params + end + + setup do + @chat = chats(:two) + @message = @chat.messages.create!( + type: "UserMessage", + content: "What is my net worth?", + ai_model: "gpt-4.1" + ) + @llm = mock + @llm.stubs(:supports_responses_endpoint?).returns(true) + @responder = Assistant::Responder.new( + message: @message, + instructions: "instructions", + function_tool_caller: Assistant::FunctionToolCaller.new([ EchoFunction.new(nil) ]), + llm: @llm + ) + end + + test "default iteration cap is eight rounds" do + assert_equal 8, Assistant::Responder::DEFAULT_MAX_TOOL_CALL_ITERATIONS + end + + test "final round requests forbid tool calls via tool_choice while keeping tool definitions" do + function_request = Provider::LlmConcept::ChatFunctionRequest.new( + id: "1", call_id: "1", function_name: "echo", function_args: "{}" + ) + tool_response = Provider::LlmConcept::ChatResponse.new( + id: "1", model: "gpt-4.1", messages: [], function_requests: [ function_request ] + ) + text_response = Provider::LlmConcept::ChatResponse.new( + id: "2", model: "gpt-4.1", + messages: [ Provider::LlmConcept::ChatMessage.new(id: "2", output_text: "Here is what I found") ], + function_requests: [] + ) + + functions_seen = [] + tool_choices_seen = [] + + @llm.stubs(:chat_response).with do |_prompt, **kwargs| + functions_seen << kwargs[:functions] + tool_choices_seen << kwargs[:tool_choice] + true + end.returns( + provider_success_response(tool_response), + provider_success_response(tool_response), + provider_success_response(text_response) + ) + + with_iteration_cap(2) do + @responder.respond + end + + # Every request carries the real tool definitions — Anthropic rejects + # messages containing tool blocks when no tools are defined — and only the + # request after the final permitted round forbids further calls. + assert_operator functions_seen.size, :>=, 2 + assert functions_seen.last.present? + assert_equal functions_seen.first, functions_seen.last + assert_equal :none, tool_choices_seen.last + assert tool_choices_seen[0..-2].all?(&:nil?) + end + + private + def with_iteration_cap(value) + previous = ENV["ASSISTANT_MAX_TOOL_CALL_ITERATIONS"] + ENV["ASSISTANT_MAX_TOOL_CALL_ITERATIONS"] = value.to_s + yield + ensure + ENV["ASSISTANT_MAX_TOOL_CALL_ITERATIONS"] = previous + end +end diff --git a/test/models/assistant_test.rb b/test/models/assistant_test.rb index 3dec5dbf4..5f3367d0c 100644 --- a/test/models/assistant_test.rb +++ b/test/models/assistant_test.rb @@ -3,6 +3,22 @@ require "test_helper" class AssistantTest < ActiveSupport::TestCase include ProviderTestHelper + test "default registry includes the analytical read tools and gates preview reads" do + default_classes = Assistant.function_classes + + assert_includes default_classes, Assistant::Function::GetMerchants + assert_includes default_classes, Assistant::Function::GetRecurringTransactions + assert_not_includes default_classes, Assistant::Function::GetInsights + assert_not_includes default_classes, Assistant::Function::GetValuations + + preview_user = users(:family_admin) + preview_user.update!(preferences: (preview_user.preferences || {}).merge("preview_features_enabled" => true)) + preview_classes = Assistant.function_classes(preview_user) + + assert_includes preview_classes, Assistant::Function::GetInsights + assert_includes preview_classes, Assistant::Function::GetValuations + end + setup do @chat = chats(:two) @message = @chat.messages.create!( diff --git a/test/models/provider/anthropic/chat_config_test.rb b/test/models/provider/anthropic/chat_config_test.rb index 8939fc76e..ad6a7ff3b 100644 --- a/test/models/provider/anthropic/chat_config_test.rb +++ b/test/models/provider/anthropic/chat_config_test.rb @@ -66,6 +66,56 @@ class Provider::Anthropic::ChatConfigTest < ActiveSupport::TestCase req[:tools].each { |t| assert_not t[:input_schema].key?(:strict) } end + test "maps tool_choice :none to the Anthropic tool_choice param when tools are present" do + config = Provider::Anthropic::ChatConfig.new( + prompt: "hi", + functions: [ + { + name: "get_net_worth", + description: "Returns net worth", + params_schema: { type: "object", properties: {}, required: [], additionalProperties: false }, + strict: true + } + ], + tool_choice: :none + ) + + req = config.build_request(model: "claude-sonnet-4-6") + + # Tool definitions must remain in the request — the API rejects messages + # containing tool blocks when no tools are defined. + assert_equal 1, req[:tools].size + assert_equal({ type: "none" }, req[:tool_choice]) + end + + test "omits tool_choice when no tools are present" do + config = Provider::Anthropic::ChatConfig.new(prompt: "hi", tool_choice: :none) + + req = config.build_request(model: "claude-sonnet-4-6") + + assert_nil req[:tools] + assert_nil req[:tool_choice] + end + + test "omits tool_choice when tool_choice is nil" do + config = Provider::Anthropic::ChatConfig.new( + prompt: "hi", + functions: [ + { + name: "get_net_worth", + description: "Returns net worth", + params_schema: { type: "object", properties: {}, required: [], additionalProperties: false }, + strict: true + } + ] + ) + + req = config.build_request(model: "claude-sonnet-4-6") + + assert_equal 1, req[:tools].size + assert_nil req[:tool_choice] + end + test "strips both symbol and string-keyed `strict` flags from input_schema" do config = Provider::Anthropic::ChatConfig.new( prompt: "hi", diff --git a/test/models/provider/openai_test.rb b/test/models/provider/openai_test.rb index 2db1013a4..652716c6a 100644 --- a/test/models/provider/openai_test.rb +++ b/test/models/provider/openai_test.rb @@ -377,6 +377,47 @@ class Provider::OpenaiTest < ActiveSupport::TestCase end end + test "history budget subtracts the real instructions estimate when given" do + Setting.stubs(:llm_max_response_tokens).returns(nil) + + with_env_overrides( + "LLM_CONTEXT_WINDOW" => "8192", + "LLM_MAX_RESPONSE_TOKENS" => nil, + "LLM_SYSTEM_PROMPT_RESERVE" => nil, + "LLM_MAX_HISTORY_TOKENS" => nil + ) do + subject = Provider::Openai.new("test-token") + instructions = "a" * 4000 + estimate = Assistant::TokenEstimator.estimate(instructions) + + assert_equal 8192 - 512 - estimate, subject.max_history_tokens(instructions: instructions) + # Without instructions the flat reserve still applies + assert_equal 8192 - 512 - 256, subject.max_history_tokens + end + end + + test "response cap is only sent when explicitly configured" do + with_env_overrides("LLM_MAX_RESPONSE_TOKENS" => nil) do + Setting.stubs(:llm_max_response_tokens).returns(nil) + subject = Provider::Openai.new("test-token") + + assert_nil subject.explicit_max_response_tokens + end + + with_env_overrides("LLM_MAX_RESPONSE_TOKENS" => nil) do + Setting.stubs(:llm_max_response_tokens).returns(768) + subject = Provider::Openai.new("test-token") + + assert_equal 768, subject.explicit_max_response_tokens + end + + with_env_overrides("LLM_MAX_RESPONSE_TOKENS" => "900") do + subject = Provider::Openai.new("test-token") + + assert_equal 900, subject.explicit_max_response_tokens + end + end + test "budget readers fall back to Setting when ENV unset" do with_env_overrides( "LLM_CONTEXT_WINDOW" => nil,