From 25a9011f14276854cd8add0624acf1ea57d4ee02 Mon Sep 17 00:00:00 2001 From: Brandon Date: Fri, 21 Aug 2026 12:58:47 -0700 Subject: [PATCH] feat(ai): analytical tool-set upgrade for the builtin assistant (#3064) * fix(assistant): survive tool failures with error and hint results instead of aborting the turn A tool exception used to raise FunctionExecutionError out of the responder loop, turning the whole turn into a generic chat error banner. An unknown tool name was worse: the rescue block itself crashed (fn.name on nil). Tool failures now come back to the model as data ({error, hint}) so the conversation survives and the model can retry once with corrected arguments. The catch-all branch logs and tells the model not to retry. FunctionExecutionError remains defined for API compatibility. * fix(assistant): strict schemas declare every property as required get_categories and get_tags declared an optional page property while inheriting strict mode, which is invalid under strict function calling (every property must be listed in required). Both now opt out of strict mode like every other paginated tool, and gain a page_size param (1..100) while being touched. A registry-walking test asserts the invariant for every current and future tool, preview tools included. * fix(assistant): HistoryTrimmer always keeps the newest turn Trimming iterates newest-first and stopped at the first group over budget. When the newest group alone exceeded the budget, everything was dropped, including the user message the model was being asked to answer, and the provider received only the system prompt. The newest group now always survives. * perf(assistant): compact AI time series and make account history opt-in get_accounts shipped a 5-year monthly series for every account on every call, roughly 60 formatted money strings per account, which dominates the tool payload for multi-account families and swamps small self-hosted context windows. The series is now opt-in (include_balance_series) and bounded by a named period (series_period, default last_365_days). to_ai_time_series states the currency once and emits numeric values instead of formatting every point; the system prompt already tells the model how to render currency. get_accounts also now returns account ids (they are what other tools accept as account_ids filters) and respects the visible scope, so hidden accounts no longer leak into responses. * refactor(assistant): id and name filters replace user-data enums in get_transactions The schema inlined every account, category, merchant, and tag name as enum values on every request. That grows without bound with family data, defeats provider prompt caching (definitions change whenever a name does), and is the pattern that made empty-enum pruning necessary in the first place. Filters are now plain string arrays documented as exact names from the sibling get_* tools, which Transaction::Search already resolves server-side, plus an account_ids UUID filter. New params: page_size (1..100), sort_by amount, types (income/expense/transfer, the way to exclude transfers), and statuses (pending/confirmed). The three now-unused enum helpers are removed from the base class; family_tag_names stays for update_tag, which still identifies tags by name. * feat(assistant): add get_merchants and get_recurring_transactions Merchants were unreachable: names appeared nowhere and update_transaction's merchant_id had no source of ids, making it unusable. get_merchants lists id, exact name, and source, scoped through available_merchants_for so merchants seen only in accounts hidden from the user never leak. Recurring transactions had a model, an Upcoming view, and no assistant reach. get_recurring_transactions lists detected and manual recurring items (status filter defaulting to active, optional upcoming_within_days window) with per-currency totals of active non-transfer items, answering subscription and upcoming-bill questions directly instead of via transaction paging. * feat(assistant): flexible periods on get_balance_sheet and trends on get_income_statement get_balance_sheet was hard-wired to five years of monthly history with no parameters, although Period supports arbitrary ranges and the chart builder takes any interval. It now accepts a named period or custom dates plus an interval, with a 400-point cap so a day-granularity request over a decade returns an error instead of a giant series. The default call is byte-compatible with the old shape. The balance sheet object is also memoized; it was being constructed four times per call. get_income_statement gains the analysis surface the assistant lacked: group_by month for a monthly income/expenses/net series (capped at 36 buckets), compare_previous_period for an equal-length prior window with absolute and percent deltas, and account_ids to scope totals to specific accounts via IncomeStatement#totals_for. Category breakdowns are family-wide by construction, so the account-filtered view omits them and says why. Unknown or inaccessible account ids come back as a soft failure naming the ids so the model can correct itself. * feat(assistant): preview reads for insights and valuations The Insights feed is generated nightly with pre-computed numbers, and the chat assistant could not read a word of it. get_insights returns the visible feed (type filter, acknowledged toggle, limit) without marking anything read; an assistant read is not the user viewing the feed. It sits in PREVIEW_FUNCTION_CLASSES because the feature itself is preview-gated, which also keeps it off the default /mcp surface. record_valuation was write-only: an agent recording provenance-cited valuations had no way to audit what it wrote or find dates already carrying a value. get_valuations lists valuation entries newest first with kind and the citation notes, scoped to accessible visible accounts. * feat(assistant): cache-stable system prompt with session context The prompt interpolated currency mid-text and the date near the end, so no two requests shared a cacheable prefix, and it told the model nothing about the family: not one account name, not a single category. Models opened most conversations blind, either wandering through tools or answering without data. The prompt is now STATIC_INSTRUCTIONS, a frozen constant that is byte-identical for every request (providers discount an exactly-repeated prefix; tool definitions are also stable now that schemas carry no user data), followed by a trailing Session context block holding everything volatile: date, date format, currency details, an account roster with balances, and category names. The static half gains a request-classification rule (CHAT / LOOKUP / ANALYSIS), a reuse-what-you-have rule with an explicit re-fetch carve-out, specific-tool preference, and the error/hint retry-once rule that pairs with the tool soft-fail contract. Context stays cheap by construction: the roster collapses to per-type counts beyond 25 accounts, categories to a count beyond 60 names, and both collapse whenever the configured context window is under 4096 (the self-hosted default is 2048), via the new Assistant::TokenBudget helper. Intro chats are untouched. * feat(assistant): raise tool-round cap to 8 with a no-tools grace turn; instructions-aware history budget Five rounds was tight for a tool surface that now supports real analysis chains, and hitting the cap raised ToolCallLimitError, which surfaced to the user as a dead chat with an error banner. The default is now eight rounds (env override unchanged), and on the final permitted round the follow-up request offers no tools, so the model must answer in text with whatever it gathered. The limit error remains as a defensive backstop. The generic-path history budget reserved a flat 256 tokens for a system prompt that already estimates well past that; the trimmer now budgets against the actual instructions when available. LLM_MAX_RESPONSE_TOKENS was reserved in budget math but never sent to the provider. It is now sent (max_tokens on chat completions, max_output_tokens on the Responses API) only when explicitly configured via ENV or a stored Setting; stock installs keep today's uncapped behavior. * test(evals): chat golden v2 exercising the real prompt and registry The eval runner scored a fiction: hardcoded instructions and four fake permissive tool schemas, so a prompt or registry regression could sail through green. It now runs STATIC_INSTRUCTIONS plus a fixed synthetic session context and builds definitions from Assistant.function_classes against a reference user (classes whose schema cannot build are skipped with a log line, never faked). chat_golden_v2 adds routing scenarios the upgrade cares about: CHAT classification must use no tools, aggregates route to get_income_statement / get_balance_sheet rather than transaction paging, and the new analytical tools are selected with sensible params. The dataset header documents the harness's single-shot limitation. * docs(ai,mcp): current tool tables, responder loop, prompt structure, timeout math Both docs listed 7 tools against a registry of 19, in three separate drift-prone copies. mcp.md now carries the canonical tables (default + preview); ai.md links to them from the MCP section, keeps one grouped functions list for the architecture chapter, and replaces its stale hardcoded registry snippet with a pointer to assistant.rb. The architecture section gains the contracts contributors need when adding a function: the responder loop (rounds vs calls, cap 8, the no-tools grace turn) and the error/hint soft-failure convention, plus the prompt's static/session-context split and its collapse gates. Timeout guidance is recomputed for the new default cap. * fix(assistant): address automated review findings Codex and CodeRabbit findings on the initial push, all verified before changing anything: - AI time series rounded every value to two decimals, which turns 0.001 BTC into 0.0; values now round to the currency's own precision (BTC 8, CLF 4, OMR 3). - get_income_statement validated account_ids against all visible accounts, but totals_for excludes hidden, excluded-from-reports and tax-advantaged accounts, so those ids produced silent zeros. Ids now validate against income_statement.eligible_accounts and the soft failure explains eligibility. - get_recurring_transactions computed totals from the displayed rows, so past the 200-row cap the value labeled a total was partial. Totals now aggregate over the full filtered scope in SQL, and the response carries total_results and a truncated flag. The upcoming_within_days window also starts at today, matching its documentation; overdue items appear in unwindowed calls. - get_valuations silently dropped a malformed date filter and presented unfiltered data as filtered; malformed dates now return invalid_date. - get_balance_sheet returned a generic failure for a reversed custom range because Period's own validation raises past the Date::Error rescue; it now returns the structured invalid_date error. - get_insights documents that its family-wide scope matches the web feed exactly (InsightsController serves Current.family.insights to every member), so the tool exposes nothing the /insights page does not already show the same user. - Tests: limit clamp proven against more insights than the cap, Setting fallbacks stubbed in the provider budget tests, currency precision and reversed-range regression tests added. * refactor(assistant): apply reviewer nitpicks - order declares type alongside its enum, matching sort_by - page-size clamp deduplicated into the base class (MAX_PAGE_SIZE + shared resolved_page_size); dead per-tool copies removed - get_accounts preloads balance rows only when the series is requested - get_income_statement validates the bucket count before running any aggregation work Deliberately unchanged: the balance sheet's monthly_history key. The default response shape stays byte-compatible for existing MCP consumers, and the nested series already states its interval. * fix(assistant): second-round review findings on get_valuations - A reversed date range (start after end) now returns the structured invalid_date error instead of presenting an empty result as filtered data, matching get_balance_sheet's handling. - Page numbers are normalized before pagination: Pagy raises on zero, negative or non-numeric pages. The fix lands as a shared resolved_page helper on the base class and applies to every paginated tool (categories, tags, merchants, transactions, holdings, valuations), since all shared the same page-or-1 pattern; schemas declare minimum: 1. * fix(assistant): round series amounts as BigDecimal before Float conversion Converting to Float first can perturb the value at the requested precision; round the exact decimal, then convert for JSON. * fix(assistant): address maintainer review findings - get_accounts no longer fails the whole listing when one account's start date lies beyond the requested period (start_date derives from the first entry, which can be future-dated); that account simply has no series. The unrescued Period.custom was reachable exactly there. - The balances preload is gone: the series goes through Balance::ChartSeriesBuilder, which runs its own query keyed by account ids, so the eager-loaded rows were loaded and discarded. - Provider::Openai#context_window now delegates to Assistant::TokenBudget, removing the duplicated ENV > Setting > default precedence so prompt assembly and the provider can never disagree about the window. * fix(ai): final no-tools round uses tool_choice none instead of dropping tools Anthropic rejects requests whose messages contain tool_use blocks when no tools are defined, so re-requesting with an empty tool list made the final-round grace die in a provider 400 on Anthropic models. The final round now sends the real tool definitions with tool_choice none, which both providers accept, and the model answers in prose as intended. * fix(assistant): scope every income statement read to the requesting user get_income_statement validated account_ids against the user-scoped statement but computed every total from an unscoped one. IncomeStatement falls back to Current.user, which is nil in the assistant job and the MCP endpoint, so the unscoped reads dropped the included_in_finances_for filter and reported family-wide totals next to ids that had been checked against a narrower set. Route all reads through one memoized user-scoped statement, the idiom get_balance_sheet already uses. Also lets the per-instance memoization in IncomeStatement apply across the eligibility check and the totals. Adds a regression test that fails without the change, plus a companion test asserting eligibility and totals agree on scope. Guard the strictness walk against an empty registry so it cannot silently assert nothing. --- app/models/assistant.rb | 10 +- app/models/assistant/configurable.rb | 169 +++++++++++----- app/models/assistant/function.rb | 50 +++-- app/models/assistant/function/get_accounts.rb | 72 ++++++- .../assistant/function/get_balance_sheet.rb | 100 +++++++-- .../assistant/function/get_categories.rb | 25 ++- app/models/assistant/function/get_holdings.rb | 3 +- .../function/get_income_statement.rb | 191 ++++++++++++++++-- app/models/assistant/function/get_insights.rb | 79 ++++++++ .../assistant/function/get_merchants.rb | 78 +++++++ .../function/get_recurring_transactions.rb | 115 +++++++++++ app/models/assistant/function/get_tags.rb | 25 ++- .../assistant/function/get_transactions.rb | 113 ++++++----- .../assistant/function/get_valuations.rb | 112 ++++++++++ app/models/assistant/function_tool_caller.rb | 35 +++- app/models/assistant/history_trimmer.rb | 6 +- app/models/assistant/responder.rb | 25 ++- app/models/assistant/token_budget.rb | 26 +++ app/models/eval/runners/chat_runner.rb | 47 +++-- app/models/provider/anthropic.rb | 2 + app/models/provider/anthropic/chat_config.rb | 10 +- app/models/provider/llm_concept.rb | 1 + app/models/provider/openai.rb | 49 ++++- db/eval_data/chat_golden_v2.yml | 132 ++++++++++++ docs/hosting/ai.md | 123 ++++++----- docs/hosting/mcp.md | 30 +-- test/models/assistant/configurable_test.rb | 60 ++++++ .../assistant/function/get_accounts_test.rb | 87 ++++++++ .../function/get_balance_sheet_test.rb | 51 +++++ .../assistant/function/get_categories_test.rb | 16 ++ .../function/get_income_statement_test.rb | 184 +++++++++++++++++ .../assistant/function/get_insights_test.rb | 81 ++++++++ .../assistant/function/get_merchants_test.rb | 53 +++++ .../get_recurring_transactions_test.rb | 79 ++++++++ .../assistant/function/get_tags_test.rb | 16 ++ .../function/get_transactions_test.rb | 48 +++++ .../assistant/function/get_valuations_test.rb | 92 +++++++++ .../function/schema_strictness_test.rb | 34 ++++ test/models/assistant/function_test.rb | 11 + .../assistant/function_tool_caller_test.rb | 83 ++++++++ test/models/assistant/history_trimmer_test.rb | 11 + test/models/assistant/responder_test.rb | 81 ++++++++ test/models/assistant_test.rb | 16 ++ .../provider/anthropic/chat_config_test.rb | 50 +++++ test/models/provider/openai_test.rb | 41 ++++ 45 files changed, 2437 insertions(+), 285 deletions(-) create mode 100644 app/models/assistant/function/get_insights.rb create mode 100644 app/models/assistant/function/get_merchants.rb create mode 100644 app/models/assistant/function/get_recurring_transactions.rb create mode 100644 app/models/assistant/function/get_valuations.rb create mode 100644 app/models/assistant/token_budget.rb create mode 100644 db/eval_data/chat_golden_v2.yml create mode 100644 test/models/assistant/function/get_accounts_test.rb create mode 100644 test/models/assistant/function/get_balance_sheet_test.rb create mode 100644 test/models/assistant/function/get_income_statement_test.rb create mode 100644 test/models/assistant/function/get_insights_test.rb create mode 100644 test/models/assistant/function/get_merchants_test.rb create mode 100644 test/models/assistant/function/get_recurring_transactions_test.rb create mode 100644 test/models/assistant/function/get_valuations_test.rb create mode 100644 test/models/assistant/function/schema_strictness_test.rb create mode 100644 test/models/assistant/responder_test.rb 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,