diff --git a/app/controllers/mcp_controller.rb b/app/controllers/mcp_controller.rb index b88349395..32243d0e5 100644 --- a/app/controllers/mcp_controller.rb +++ b/app/controllers/mcp_controller.rb @@ -111,8 +111,13 @@ class McpController < ApplicationController { content: [ { type: "text", text: result.to_json } ] } rescue => e - Rails.logger.error "MCP tools/call error: #{e.message}" - { content: [ { type: "text", text: { error: e.message }.to_json } ], isError: true } + Rails.logger.error "MCP tools/call error: #{e.class}: #{e.message}" + + # Whatever the tool raised, its message was written for a log, not for an + # external client: a RecordNotFound carries the access-control SQL, and a + # PG range error carries the column definition. The full text stays in the + # log above, where an operator can read it. + { content: [ { type: "text", text: { error: "The tool failed to run", tool: name }.to_json } ], isError: true } end def authenticate_mcp_token! diff --git a/app/models/assistant.rb b/app/models/assistant.rb index 1635b67b5..7473d3ec2 100644 --- a/app/models/assistant.rb +++ b/app/models/assistant.rb @@ -6,11 +6,14 @@ module Assistant "external" => Assistant::External }.freeze - # Statement Vault + provenance tools, for users who opted into preview features - # in Settings -> Preferences. They back the wealth agent-harness workflow + # Tools for users who opted into preview features in Settings -> Preferences. + # + # Statement Vault + provenance tools back the wealth agent-harness workflow # 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. + # + # The bills tools back the preview-gated Bills subsystem. PREVIEW_FUNCTION_CLASSES = [ Function::UploadAccountStatement, Function::ListAccountStatements, @@ -18,7 +21,18 @@ module Assistant Function::GetStatementCoverage, Function::RecordValuation, Function::GetValuations, - Function::GetInsights + Function::GetInsights, + # Bills: the whole subsystem is preview-gated, so its tools ride the same + # per-user flag as the surfaces they operate on. Each tool additionally + # re-checks the family's recurring feature gate and the user's + # account-access scope itself. + Function::GetBills, + Function::GetBillDetails, + Function::GetPaycheckPlan, + Function::GetBillAudit, + Function::CreateBill, + Function::UpdateBill, + Function::RecordBillPayment ].freeze class << self diff --git a/app/models/assistant/configurable.rb b/app/models/assistant/configurable.rb index 1c16229ce..8cae6b8e0 100644 --- a/app/models/assistant/configurable.rb +++ b/app/models/assistant/configurable.rb @@ -28,6 +28,7 @@ module Assistant::Configurable - 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. + - Never mention internal tool or function names in your responses. Describe what you did in plain language ("I checked your bills", not "I called get_bills"). - 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 diff --git a/app/models/assistant/function/bills_support.rb b/app/models/assistant/function/bills_support.rb new file mode 100644 index 000000000..adf955c73 --- /dev/null +++ b/app/models/assistant/function/bills_support.rb @@ -0,0 +1,158 @@ +# frozen_string_literal: true + +# Shared plumbing for the Bills tools. The Bills pages sit behind a per-family +# feature gate and a per-user account-access scope; tool calls never pass +# through those controllers, so every tool re-checks both here. +# +# Status vocabulary: these tools speak the UI's lifecycle words, not raw +# storage. Nothing in the app writes the stored value "paused" (the Pause +# button stores "inactive"), so "paused" here means the inactive+paused set, +# exactly as the All-bills filter treats it. +module Assistant::Function::BillsSupport + # Mirrors BillsController::LIFECYCLE_STATUSES with the two review states. + STATUS_VOCABULARY = { + "active" => %w[active], + "suggested" => %w[suggested], + "paused" => %w[inactive paused], + "ended" => %w[ended] + }.freeze + + private + def recurring_disabled? + family.recurring_transactions_disabled? + end + + def recurring_disabled_result + { + error: "Bills & recurring transactions are disabled for this family", + hint: "Do not retry. Tell the user this feature is switched off under Settings -> Recurring transactions, and answer from transaction data instead." + } + end + + def accessible_series + family.recurring_transactions.accessible_by(user) + end + + def find_series(id) + unless valid_uuid?(id) + return [ nil, { + error: "bill_id is not a valid id", + hint: "Pass the exact id returned by get_bills." + } ] + end + + # find (not find_by): a missing or foreign id raises RecordNotFound, + # which the tool caller converts into an error+hint result. + [ accessible_series.find(id), nil ] + end + + # The write tools' lookup. Reading a shared bill is fine; changing it is + # not: sharing is per account, so a read-only share must not mutate the + # series, exactly as RecurringTransactionsController#ensure_series_writable + # enforces for the pages. Accountless series carry no account gate. The + # bill is already visible to this user, so naming the reason leaks nothing. + def find_writable_series(id) + series, error = find_series(id) + return [ nil, error ] if error + + if series.account_id.present? && !Account.writable_by(user).where(id: series.account_id).exists? + return [ nil, { + error: "#{series.display_name} is on an account shared with you read-only", + hint: "You can read this bill but not change it. Do not retry." + } ] + end + + [ series, nil ] + end + + def display_status(series) + case series.status + when "inactive", "paused" then "paused" + else series.status + end + end + + def serialize_series(series) + detection = RecurringTransaction::FrequencyPreset.detect(series) + + { + id: series.id, + name: series.display_name, + bill_type: series.bill_type, + status: display_status(series), + amount: series.amount_money.abs.format, + currency: series.currency, + frequency: detection.key || "custom", + next_due_date: series.next_due_date&.iso8601, + autopay: series.autopay, + detected_automatically: !series.manual, + category: series.category&.name, + account: account_ref(series.account), + destination_account: account_ref(series.destination_account), + monthly_equivalent: series.monthly_equivalent_amount&.abs&.format, + payment_url: series.payment_url + }.compact + end + + def serialize_occurrence(occurrence) + return nil if occurrence.nil? + + # Read once, derive locally: resolved_expected_amount is unmemoized and + # can cost two queries per call under the `last` amount strategy. + expected = occurrence.resolved_expected_amount + paid = occurrence.confirmed_allocated + + { + due_on: occurrence.due_on.iso8601, + effective_due_on: occurrence.effective_due_on.iso8601, + state: occurrence.derived_state.to_s, + expected: Money.new(expected, occurrence.currency).format, + paid: Money.new(paid, occurrence.currency).format, + remaining: Money.new([ expected - paid, 0 ].max, occurrence.currency).format, + partially_paid: occurrence.scheduled? && paid.positive? && paid < expected + } + end + + def account_ref(account) + return nil if account.nil? + + { id: account.id, name: account.name } + end + + # One grouped SUM for the given occurrences, injected through the same + # cache the list views use, so serialization issues no per-row queries. + def preload_allocation_sums(occurrences) + rows = occurrences.compact + return if rows.empty? + + sums = RecurringAllocation.confirmed + .where(recurring_occurrence_id: rows.map(&:id)) + .group(:recurring_occurrence_id) + .sum(:allocated_amount) + + rows.each { |occurrence| occurrence.cached_confirmed_allocated = sums[occurrence.id] || 0 } + end + + # current_occurrence resolved from the preloaded association instead of + # the model's per-series queries (100 series would mean 100 queries). + def current_occurrence_from_loaded(series) + occurrences = series.recurring_occurrences + occurrences.select(&:scheduled?).min_by(&:due_on) || + occurrences.max_by(&:due_on) + end + + # Spend commitments only: income is not spend and a transfer moves money + # rather than spending it, so both stay out of any totals row. + def spend_series?(series) + !%w[income transfer].include?(series.bill_type) + end + + # How far a price moved, as a signed percent of what it was. nil when + # there is no previous amount to compare against. + def percent_change(previous_amount, new_amount) + previous = previous_amount.abs + return nil if previous.zero? + + (((new_amount.abs - previous) / previous) * 100).round(1).to_f + end +end diff --git a/app/models/assistant/function/create_bill.rb b/app/models/assistant/function/create_bill.rb new file mode 100644 index 000000000..12c837162 --- /dev/null +++ b/app/models/assistant/function/create_bill.rb @@ -0,0 +1,190 @@ +class Assistant::Function::CreateBill < Assistant::Function + include Assistant::Function::BillsSupport + + class << self + def name + "create_bill" + end + + def description + <<~INSTRUCTIONS + Create a bill, subscription, installment plan or income schedule for the user. + + Rules: + - amount is always a positive magnitude; set is_income true for income and the + app derives the sign. Never pass a negative amount. + - account_name must exactly match a name returned by get_accounts. Omit it to + create the bill without an account. + - category_name must exactly match a name returned by get_categories. + - first_due_on seeds the schedule: its day of month (or weekday for weekly + cadences) becomes the recurring due day. + - Transfers between accounts cannot be created here. + + Confirm the details with the user before calling this. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: %w[name amount first_due_on], + properties: { + name: { type: "string", description: "What the user calls this bill." }, + amount: { type: "number", minimum: 0.01, description: "Positive magnitude per occurrence." }, + first_due_on: { type: "string", description: "Next due date, YYYY-MM-DD." }, + frequency: { + type: "string", + enum: RecurringTransaction::FrequencyPreset::PRESETS, + description: "Cadence (default monthly)." + }, + is_income: { type: "boolean", description: "True for a paycheck/income schedule." }, + bill_type: { + type: "string", enum: %w[bill subscription installment], + description: "Kind of obligation (ignored for income)." + }, + account_name: { type: "string", description: "Exact account name from get_accounts." }, + category_name: { type: "string", description: "Exact category name from get_categories." }, + autopay: { type: "boolean" }, + payment_url: { type: "string", description: "Where this bill gets paid." }, + notes: { type: "string" } + } + ) + end + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + is_income, income_error = resolve_boolean(params, "is_income") + return income_error if income_error + + autopay, autopay_error = resolve_boolean(params, "autopay") + return autopay_error if autopay_error + + account, account_error = resolve_account(params["account_name"]) + return account_error if account_error + + category, category_error = resolve_category(params["category_name"]) + return category_error if category_error + + frequency, frequency_error = resolve_frequency(params["frequency"]) + return frequency_error if frequency_error + + series = RecurringTransaction::DeclaredBill.new( + family: family, + user: user, + attrs: { + name: params["name"], + amount: params["amount"], + first_due_on: params["first_due_on"], + frequency_preset: frequency, + is_income: is_income, + account_id: account&.id, + payment_url: params["payment_url"], + autopay: autopay, + notes: params["notes"] + } + ).build + + if series.errors.none? + series.category = category if category + if !is_income && params["bill_type"].presence_in(%w[bill subscription installment]) + series.bill_type = params["bill_type"] + end + end + + unless series.errors.none? && RecurringTransaction::DeclaredBill.save(series) + return { + error: series.errors.full_messages.to_sentence, + hint: "Fix the named fields and retry once." + } + end + + { + created: true, + bill: serialize_series(series), + upcoming_due_dates: series.schedule.occurrences_between(Date.current, Date.current + 400).first(3).map(&:iso8601) + } + end + + private + # An unrecognized cadence used to fall back to monthly. The enum word for a + # once-a-year bill is "annual", so a model offering the equally natural + # "yearly" turned a $600 premium into a $600 monthly commitment, twelve + # times the real obligation, with no indication anything had been ignored. + # A financial write is the wrong place to guess. + def resolve_frequency(value) + return [ "monthly", nil ] if value.blank? + + preset = value.to_s.presence_in(RecurringTransaction::FrequencyPreset::PRESETS) + return [ preset, nil ] if preset + + [ nil, { + error: "#{value} is not a frequency this app recognizes", + hint: "Use one of: #{RecurringTransaction::FrequencyPreset::PRESETS.join(', ')}. " \ + "Omit it entirely for monthly." + } ] + end + + # Writable, not merely visible: attaching a bill to an account changes + # what that account's owners see, so a read-only share is not a + # destination. Namesakes are refused rather than picked between: a + # financial write is the wrong place to guess. + def resolve_account(name) + return [ nil, nil ] if name.blank? + + matches = Account.writable_by(user).where(name: name).limit(2).to_a + return [ matches.first, nil ] if matches.size == 1 + + if matches.empty? + [ nil, { + error: "No account named #{name.inspect} that you can add bills to", + hint: "Call get_accounts and retry once with the exact name of a writable account." + } ] + else + [ nil, { + error: "More than one account is named #{name.inspect}", + hint: "Ask the user which one they mean; this tool cannot pick between namesakes." + } ] + end + end + + # Category namesakes cannot exist: names are unique per family + # (index_categories_on_family_id_and_name), so find_by is unambiguous. + def resolve_category(name) + return [ nil, nil ] if name.blank? + + category = family.categories.find_by(name: name) + return [ category, nil ] if category + + [ nil, { + error: "No category named #{name.inspect}", + hint: "Call get_categories and retry once with the exact category name." + } ] + end + + # The tool caller does not enforce params_schema, so a string "true" from + # a loose MCP client would otherwise compare unequal to true and silently + # flip a paycheck into a bill. An explicit allowlist, not + # ActiveModel::Type::Boolean, because that cast reads every unrecognized + # string as true, and a financial write is the wrong place to guess. + TRUTHY_INPUTS = [ true, "true", "t", "1", 1 ].freeze + FALSY_INPUTS = [ false, "false", "f", "0", 0 ].freeze + + def resolve_boolean(params, key) + value = params[key] + return [ false, nil ] if value.nil? + + normalized = value.is_a?(String) ? value.strip.downcase : value + return [ true, nil ] if TRUTHY_INPUTS.include?(normalized) + return [ false, nil ] if FALSY_INPUTS.include?(normalized) + + [ nil, { + error: "#{key} must be true or false", + hint: "Pass a JSON boolean, not #{value.inspect}." + } ] + end +end diff --git a/app/models/assistant/function/get_bill_audit.rb b/app/models/assistant/function/get_bill_audit.rb new file mode 100644 index 000000000..59330782c --- /dev/null +++ b/app/models/assistant/function/get_bill_audit.rb @@ -0,0 +1,182 @@ +class Assistant::Function::GetBillAudit < Assistant::Function + include Assistant::Function::BillsSupport + + SECTION_LIMIT = 20 + NOTICE_WINDOW_DAYS = 30 + + class << self + def name + "get_bill_audit" + end + + def description + <<~INSTRUCTIONS + Audit the user's bills and subscriptions and return the facts a review needs: + possible duplicate bills, recent price changes, trials about to convert, + upcoming renewals, bills overdue by at least one whole billing cycle, paused + bills still carrying unpaid occurrences, detections awaiting the user's + confirmation, and recurring charge patterns the user has not declared yet. + + Every section is computed deterministically from the user's data; narrate and + prioritize the findings rather than recomputing them. Duplicate detection is + deliberately strict (same name, amount and due day), so two subscription tiers + to one merchant are never flagged as duplicates. Propose specific fixes and ask + before changing anything. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + lookback_months: { + type: "integer", minimum: 1, maximum: 24, + description: "How far back to report price changes (default 12 months)." + } + } + ) + end + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + # Same contract as get_bills' due_within_days: a present value outside the + # range is rejected, not silently adjusted to answer a different question. + if params["lookback_months"].present? + lookback = Integer(params["lookback_months"].to_s, exception: false) + unless lookback&.between?(1, 24) + return { + error: "lookback_months must be a whole number between 1 and 24", + hint: "Retry once with a value in that range, or omit it for 12." + } + end + else + lookback = 12 + end + active = accessible_series.active + .includes(:merchant, :account, :recurrence_rules) + .to_a + + { + as_of_date: Date.current.iso8601, + possible_duplicates: section(possible_duplicates(active)), + price_changes: section(price_changes(lookback)), + upcoming_trials: section(upcoming(active, :trial_ends_on)), + upcoming_renewals: section(upcoming(active, :renews_on)), + long_overdue: section(long_overdue(active)), + dormant: section(dormant), + awaiting_confirmation: section(awaiting_confirmation), + undeclared_candidates: section(undeclared_candidates) + } + end + + private + def section(items) + { items: items.first(SECTION_LIMIT), truncated: items.size > SECTION_LIMIT, count: items.size } + end + + def possible_duplicates(active) + active.group_by(&:duplicate_key) + .values + .select { |group| group.size > 1 } + .map do |group| + { + name: group.first.display_name, + amount: group.first.amount_money.abs.format, + bills: group.map { |series| { id: series.id, account: series.account&.name } } + } + end + end + + def price_changes(lookback_months) + RecurringPriceChange.joins(:recurring_transaction) + .merge(accessible_series) + .where("effective_on >= ?", lookback_months.months.ago.to_date) + .includes(:recurring_transaction) + .order(effective_on: :desc) + .map do |change| + { + bill: change.recurring_transaction.display_name, + bill_id: change.recurring_transaction_id, + effective_on: change.effective_on.iso8601, + previous_amount: Money.new(change.previous_amount, change.currency).abs.format, + new_amount: Money.new(change.new_amount, change.currency).abs.format, + percent_change: percent_change(change.previous_amount, change.new_amount) + }.compact + end + end + + def upcoming(active, date_column) + window = Date.current..(Date.current + NOTICE_WINDOW_DAYS) + + active.select { |series| window.cover?(series.public_send(date_column)) } + .sort_by { |series| series.public_send(date_column) } + .map do |series| + { + bill_id: series.id, + name: series.display_name, + date: series.public_send(date_column).iso8601, + amount: series.amount_money.abs.format + } + end + end + + # A whole billing cycle late in the series' own cadence: meaningful for + # weekly and annual bills alike, where a flat day threshold is not. + def long_overdue(active) + active.select { |series| spend_series?(series) && series.cycles_overdue >= 1 } + .sort_by { |series| -series.cycles_overdue } + .map do |series| + { + bill_id: series.id, + name: series.display_name, + cycles_overdue: series.cycles_overdue, + next_due_date: series.next_due_date&.iso8601, + amount: series.amount_money.abs.format + } + end + end + + # Paused bills still carrying open occurrences: set aside but not settled. + def dormant + accessible_series.where(status: STATUS_VOCABULARY.fetch("paused")) + .joins(:recurring_occurrences) + .merge(RecurringOccurrence.open_status) + .distinct + .map do |series| + { bill_id: series.id, name: series.display_name, status: display_status(series) } + end + end + + def awaiting_confirmation + accessible_series.suggested.order(next_expected_date: :asc).map do |series| + { bill_id: series.id, name: series.display_name, amount: series.amount_money.abs.format } + end + end + + # The same clustering the add-bill dialog offers: recurring outflow shapes + # detection spotted that no series covers yet. Patterns are family-wide, + # so they are filtered to the accounts this user can actually reach. + def undeclared_candidates + accessible_ids = Account.accessible_by(user).pluck(:id) + + RecurringTransaction::Identifier.new(family) + .candidate_patterns(sign: :outflow) + .select { |pattern| accessible_ids.include?(pattern[:account_id]) } + .sort_by { |pattern| pattern[:last_occurrence_date] } + .reverse + .map do |pattern| + { + name: pattern[:name], + average_amount: Money.new(pattern[:expected_amount_avg].abs, pattern[:currency]).format, + occurrence_count: pattern[:occurrence_count], + last_seen: pattern[:last_occurrence_date].iso8601 + } + end + end +end diff --git a/app/models/assistant/function/get_bill_details.rb b/app/models/assistant/function/get_bill_details.rb new file mode 100644 index 000000000..a57ae551f --- /dev/null +++ b/app/models/assistant/function/get_bill_details.rb @@ -0,0 +1,169 @@ +class Assistant::Function::GetBillDetails < Assistant::Function + include Assistant::Function::BillsSupport + + HISTORY_LIMIT = 12 + PRICE_CHANGE_LOOKBACK_MONTHS = 24 + + class << self + def name + "get_bill_details" + end + + def description + <<~INSTRUCTIONS + Get one bill's complete story: full configuration, every open occurrence, the last + #{HISTORY_LIMIT} settled occurrences with their payments, upcoming due dates, price-change + history, and cost analytics. + + Analytics are computed from confirmed payments on settled occurrences only, never + from estimates, and are null when nothing has been paid yet. The one figure that is + not payment-derived says so in its name: annualized_declared is the amount on the + bill times its cadence, while annualized_cost follows what has actually been paid. + Where they disagree, annualized_cost is what the bill is costing. + + history is capped at the last #{HISTORY_LIMIT} settled cycles and price_changes at + #{PRICE_CHANGE_LOOKBACK_MONTHS} months. history_window and price_change_window report + the real totals, so do not sum the rows and present the result as a lifetime figure. + + bill_id must be the exact id returned by get_bills. + INSTRUCTIONS + end + end + + def params_schema + build_schema( + required: [ "bill_id" ], + properties: { + bill_id: { type: "string", description: "The bill's id, exactly as returned by get_bills." } + } + ) + end + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + series, error = find_series(params["bill_id"]) + return error if error + + open_occurrences = series.recurring_occurrences.open_status.order(:due_on).to_a + closed = series.recurring_occurrences.closed + history = closed.order(due_on: :desc).limit(HISTORY_LIMIT).includes(allocations: :entry).to_a + preload_allocation_sums(open_occurrences + history) + + changes = price_changes(series) + + { + bill: serialize_series(series).merge(configuration(series)), + analytics: analytics(series), + open_occurrences: open_occurrences.map { |occurrence| serialize_occurrence(occurrence) }, + history: history.map { |occurrence| serialize_history_row(occurrence) }, + history_window: truncation(history.size, closed.count), + upcoming_due_dates: series.schedule.occurrences_between(Date.current + 1, Date.current + 400).first(3).map(&:iso8601), + price_changes: changes, + price_change_window: { months: PRICE_CHANGE_LOOKBACK_MONTHS, count: changes.size } + } + end + + private + def configuration(series) + { + amount_strategy: series.amount_strategy, + weekend_adjust: series.weekend_adjust, + end_mode: series.end_mode, + end_on: series.end_on&.iso8601, + end_after_count: series.end_after_count, + anchor_date: series.anchor_date&.iso8601, + notes: series.notes, + notify_days_before: series.notify_days_before, + overdue_grace_days: series.overdue_grace_days, + trial_ends_on: series.trial_ends_on&.iso8601, + renews_on: series.renews_on&.iso8601, + cancelled_on: series.cancelled_on&.iso8601, + schedule_pinned: series.schedule_pinned?, + expected_amount_min: series.expected_amount_min_money&.abs&.format, + expected_amount_max: series.expected_amount_max_money&.abs&.format, + expected_amount_avg: series.expected_amount_avg_money&.abs&.format + }.compact + end + + # Same discipline as the bill page: what each settled cycle actually cost, + # from confirmed allocations on paid occurrences. The frozen + # expected_amount is an estimate; averaging estimates beside sums of real + # payments would let the page disagree with itself. + def analytics(series) + paid_amounts = RecurringAllocation.confirmed + .joins(:recurring_occurrence) + .where(recurring_occurrences: { + recurring_transaction_id: series.id, + status: "paid" + }) + .group(:recurring_occurrence_id) + .sum(:allocated_amount) + .values + + return nil if paid_amounts.empty? + + ytd = RecurringAllocation.confirmed + .joins(:recurring_occurrence) + .where(recurring_occurrences: { recurring_transaction_id: series.id }) + .where("recurring_allocations.paid_on >= ?", Date.current.beginning_of_year) + .sum(:allocated_amount) + + average = paid_amounts.sum / paid_amounts.size + + { + average_paid: Money.new(average, series.currency).format, + lowest_paid: Money.new(paid_amounts.min, series.currency).format, + highest_paid: Money.new(paid_amounts.max, series.currency).format, + # This block is documented as payments-only, and annualized_cost was the + # exception: the DECLARED amount times cadence, sitting beside an average + # derived from what was actually paid. A bill declared at $100 whose every + # payment was $50 reported a $50 average and a $1,200 year in the same + # hash, and the description told the model to trust it as payment-derived. + # Run rate now follows the payments; the declared figure keeps its own + # name, so a caller comparing the two can see the gap. annualized_cost + # keeps its name and starts meaning what the block always promised. + annualized_cost: Money.new(average * series.schedule.occurrences_per_year, series.currency).format, + annualized_declared: (series.monthly_equivalent_amount * 12).abs.format, + paid_this_year: Money.new(ytd, series.currency).format + } + end + + # get_bills reports total_results and truncated; every get_bill_audit + # section reports {items, truncated, count}. History and price changes + # clamped silently, so an assistant summing the rows it was given reported + # a lifetime total short by however many cycles fell off the end. + def truncation(shown, total) + { count: total, truncated: total > shown } + end + + def serialize_history_row(occurrence) + serialize_occurrence(occurrence).merge( + status: occurrence.status, + payments: occurrence.allocations.map do |allocation| + { + amount: allocation.allocated_amount_money.format, + paid_on: allocation.paid_on&.iso8601, + source: allocation.source, + state: allocation.state, + transaction_name: allocation.entry&.name + }.compact + end + ) + end + + def price_changes(series) + series.recurring_price_changes + .where("effective_on >= ?", PRICE_CHANGE_LOOKBACK_MONTHS.months.ago.to_date) + .order(effective_on: :desc) + .map do |change| + { + effective_on: change.effective_on.iso8601, + previous_amount: Money.new(change.previous_amount, change.currency).abs.format, + new_amount: Money.new(change.new_amount, change.currency).abs.format, + percent_change: percent_change(change.previous_amount, change.new_amount), + source: change.source + } + end + end +end diff --git a/app/models/assistant/function/get_bills.rb b/app/models/assistant/function/get_bills.rb new file mode 100644 index 000000000..d199a1cc9 --- /dev/null +++ b/app/models/assistant/function/get_bills.rb @@ -0,0 +1,219 @@ +class Assistant::Function::GetBills < Assistant::Function + include Assistant::Function::BillsSupport + + MAX_RESULTS = 100 + + class << self + def name + "get_bills" + end + + def description + <<~INSTRUCTIONS + Get the user's bills, subscriptions and other recurring obligations, each with its + current occurrence's payment state. + + Key concepts: + - status is the series lifecycle: "active" (default), "suggested" (detected from + bank data, awaiting the user's confirmation -- not yet a real bill), + "paused" (set aside by the user), "ended" (dismissed or finished), "all". + - payment_state filters on the CURRENT occurrence instead: "overdue", "due", + "upcoming", "partial" (partly paid), "paid". + - bill_type "income" is a declared income schedule (paycheck), not an obligation. + Amounts are always positive magnitudes; bill_type carries the direction. + - totals exclude income and transfers (a transfer moves money, it is not spend), + and normalize each bill to a monthly equivalent whatever its cadence. + + Use get_bill_details for one bill's full history and configuration. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + status: { + type: "string", + enum: %w[active suggested paused ended all], + description: "Series lifecycle filter. Defaults to active." + }, + payment_state: { + type: "string", + enum: %w[overdue due upcoming partial paid], + description: "Filter by the current occurrence's payment state (applied after the status filter)." + }, + bill_type: { + type: "string", + enum: RecurringTransaction.bill_types.keys, + description: "Filter by kind of recurring obligation." + }, + search: { type: "string", description: "Substring match on the bill or merchant name." }, + due_within_days: { + type: "integer", minimum: 1, maximum: 365, + description: "Only bills whose next due date falls within this many days." + } + } + ) + end + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + # recurrence_rules included because serialization reads the schedule + # (frequency, next due date); without it every series costs one query. + scope = accessible_series.includes(:merchant, :account, :destination_account, :category, + :recurring_occurrences, :recurrence_rules) + invalid = reject_unknown_filters(params) + return invalid if invalid + + scope = apply_status_filter(scope, params["status"]) + + if (bill_type = params["bill_type"]).present? + scope = scope.where(bill_type: bill_type) + end + + if (search = params["search"].to_s).present? + pattern = "%#{ActiveRecord::Base.sanitize_sql_like(search)}%" + scope = scope.left_joins(:merchant) + .where("recurring_transactions.name ILIKE :p OR merchants.name ILIKE :p", p: pattern) + end + + rows = scope.order(next_expected_date: :asc).to_a + currents = rows.index_with { |series| current_occurrence_from_loaded(series) } + preload_allocation_sums(currents.values) + + # Integer(..., exception: false): MCP clients send whatever they like. A + # filter that silently answers a different question than it was asked is + # worse than an error, so out-of-range values are rejected, not adjusted. + if params["due_within_days"].present? + days = Integer(params["due_within_days"].to_s, exception: false) + unless days&.between?(1, 365) + return { + error: "due_within_days must be a whole number between 1 and 365", + hint: "Retry once with a value in that range, or omit it." + } + end + + horizon = Date.current + days + rows = rows.select { |series| series.next_due_date.present? && series.next_due_date <= horizon } + end + + if (state = params["payment_state"]).present? + rows = rows.select { |series| payment_state_matches?(currents[series], state) } + end + + shown = rows.first(MAX_RESULTS) + + { + as_of_date: Date.current.iso8601, + total_results: rows.size, + truncated: rows.size > shown.size, + family_currency: family.currency, + bills: shown.map { |series| serialize_series(series).merge(current_occurrence: serialize_occurrence(currents[series])) }, + totals: totals_over(rows, currents) + }.merge(rows.empty? ? { hint: other_status_hint(params) }.compact : {}) + end + + private + PAYMENT_STATES = %w[overdue due upcoming partial paid].freeze + + # strict_mode? is false and MCP clients bypass provider validation + # entirely, so out-of-schema values arrive here routinely. Each one used to + # fail differently and silently: an unknown status was coerced to active, + # an unknown bill_type dropped its filter so "subscriptions" (plural) + # answered with rent and a car loan totalled as subscriptions, and an + # unknown payment_state matched nothing and read as an empty family. + # + # A value the caller believes in is worth an error. Guessing at it produces + # a confident answer about the wrong bills. + def reject_unknown_filters(params) + unknown_value(params["status"], STATUS_VOCABULARY.keys + [ "all" ], "status") || + unknown_value(params["bill_type"], RecurringTransaction.bill_types.keys, "bill_type") || + unknown_value(params["payment_state"], PAYMENT_STATES, "payment_state") + end + + def unknown_value(value, allowed, field) + return nil if value.blank? || value.to_s.in?(allowed) + + { + error: "#{value} is not a valid #{field}", + hint: "Valid values: #{allowed.join(', ')}." + } + end + + def apply_status_filter(scope, status) + return scope if status == "all" + + scope.where(status: STATUS_VOCABULARY.fetch(status.presence || "active", STATUS_VOCABULARY.fetch("active"))) + end + + # An empty answer is ambiguous: it reads as "you have none of these" when it + # usually means "every one you have sits under a status this call filtered + # out". Detection parks new series in `suggested`, so asking about + # subscriptions on a family that has confirmed none answers zero results and + # a $0 total, which is worse than no answer at all. Only runs when nothing + # matched, so the normal path costs no extra query. + def other_status_hint(params) + # Only the status filter can be pointed elsewhere. When something else + # emptied the result, saying "nothing matched status active" is simply + # false, and the retry it prescribes returns empty again with no hint at + # all, because this method early-returns on status: all. + narrowing = params.values_at("payment_state", "search", "due_within_days").compact_blank + return if narrowing.any? + + requested = params["status"].presence || "active" + return if requested == "all" + + known = STATUS_VOCABULARY.fetch(requested, STATUS_VOCABULARY.fetch("active")) + scope = accessible_series.where.not(status: known) + + if (bill_type = params["bill_type"]).presence_in(RecurringTransaction.bill_types.keys) + scope = scope.where(bill_type: bill_type) + end + + elsewhere = scope.group(:status).count + return if elsewhere.empty? + + summary = elsewhere.sort_by { |_status, count| -count } + .map { |status, count| "#{count} #{status}" } + .join(", ") + + "Nothing matched status #{requested}, but this family has #{summary}. Call get_bills " \ + "again with that status, or status: all, before telling the user they have none." + end + + def payment_state_matches?(occurrence, state) + return false if occurrence.nil? + + case state + when "overdue" then occurrence.overdue? + when "due" then occurrence.derived_state == :due + when "upcoming" then occurrence.derived_state == :upcoming + when "partial" then occurrence.partially_paid? + when "paid" then occurrence.paid? + else false + end + end + + # Over the FULL filtered set, not the shown page, so a truncated list + # still reports honest totals. + def totals_over(rows, currents) + spend = rows.select { |series| series.active? && spend_series?(series) } + + monthly = spend.group_by(&:currency).to_h do |currency, group| + total = group.sum(Money.new(0, currency)) { |series| series.monthly_equivalent_amount.abs } + [ currency, total.format ] + end + + { + active_count: rows.count(&:active?), + overdue_count: rows.count { |series| currents[series]&.overdue? }, + active_monthly_equivalent_by_currency: monthly + } + end +end diff --git a/app/models/assistant/function/get_paycheck_plan.rb b/app/models/assistant/function/get_paycheck_plan.rb new file mode 100644 index 000000000..32bf56f20 --- /dev/null +++ b/app/models/assistant/function/get_paycheck_plan.rb @@ -0,0 +1,159 @@ +class Assistant::Function::GetPaycheckPlan < Assistant::Function + include Assistant::Function::BillsSupport + + class << self + def name + "get_paycheck_plan" + end + + def description + <<~INSTRUCTIONS + Get the user's income plan: time sliced into pay periods by their declared income + schedule, with each period showing what is due before the next payday, what must + stay reserved for bigger bills due later, and what is genuinely safe to spend + (income - due - reserved). + + Key concepts: + - Only income the user declared defines paydays. Detected bank inflows never do. + - A "bridge" period is the window between today and the next payday: nothing + arrives in it, so what it needs must come from cash already in hand. + - "reserved" is the part of a later bill that its own paycheck cannot cover, + set aside out of an earlier one -- rent that outgrows one paycheck reserves + the difference from the paychecks just before it. A bill its own paycheck + covers reserves nothing. + - A "short" period's obligations exceed its income by "shortfall". + + This answers "can I afford X before my next paycheck" and "which paycheck does + this bill come out of". + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [], + properties: { + periods_limit: { + type: "integer", minimum: 1, maximum: 6, + description: "How many pay periods to plan (default 3)." + } + } + ) + end + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + planner = RecurringTransaction::PaycheckPlanner.new(family, user: user) + limit = (Integer(params["periods_limit"].to_s, exception: false) || 3).clamp(1, 6) + periods = planner.plan(periods_limit: limit) + + if periods.blank? + return { + error: "No declared income schedule", + hint: "Only manually declared income defines paydays; detected inflows never do. Suggest the user adds their income under Bills -> Income plan. Do not infer paydays from transaction data." + } + end + + { + as_of_date: Date.current.iso8601, + family_currency: family.currency, + unconvertible_count: planner.unconvertible_count, + periods: periods.map { |period| serialize_period(period) } + }.merge(unconfirmed_exclusion) + end + + private + # The planner counts confirmed series only, which is the right call: a + # detection nobody has acknowledged is not yet an obligation. But every + # figure here is spending headroom, so dropping them without a word makes + # the plan read more comfortable than it is. Name what was left out and let + # the assistant caveat the number instead of overstating it. + def unconfirmed_exclusion + # Spend only. This counted every suggested series including income, and + # then asserted that safe-to-spend is an upper bound. An excluded + # paycheck pushes it the other way, so a family whose only pending + # detection was income was told its headroom was overstated when the + # opposite was true. + count = accessible_series.suggested.where.not(bill_type: "income").count + return {} if count.zero? + + { + unconfirmed_excluded: { + count: count, + note: "#{count} detected series are still awaiting confirmation and are NOT counted in " \ + "these figures, so safe-to-spend is an upper bound. Say so when presenting it. " \ + "Call get_bills with status: suggested to list them." + } + } + end + + # A bridge window earns nothing, so income minus obligations is negative + # whenever a bill falls in it. Reporting that as safe_after_bills told the + # assistant the user was underwater on a window that is funded from cash + # already in the bank, and it read as a deficit next to short: false. + # + # For a bridge, headroom is cash minus what is due out of it. When the + # balance cannot be read there is no honest number, so the key is OMITTED + # from the payload (serialize_period compacts nils away), which the + # unreadable-balance test pins on purpose: an absent key cannot be read + # aloud as a figure. + def safe_after_bills(period) + return period.cash_after_obligations.nil? ? nil : fmt(period.cash_after_obligations) if period.bridge? + + # A short window has no safe amount. The page prints the shortfall under + # its own label and never renders a negative "safe"; the tool emitted + # -$6,300.00 as safe_after_bills, which read aloud is not a sentence + # anybody means. short and shortfall carry that case already. + return nil if period.short? + + fmt(period.remaining) + end + + def serialize_period(period) + { + starts_on: period.starts_on.iso8601, + ends_on: period.ends_on.iso8601, + bridge: period.bridge?, + income: fmt(period.income), + income_sources: period.income_sources, + due_total: fmt(period.due_total), + reserved_total: fmt(period.reserved_total), + safe_after_bills: safe_after_bills(period), + cash_on_hand: (period.bridge? && period.cash_on_hand.present? ? fmt(period.cash_on_hand) : nil), + short: period.short?, + shortfall: period.short? ? fmt(period.shortfall) : nil, + bills_due: period.items_due.map { |item| serialize_item(item) }, + reserved_for_later: period.items_reserved.map { |item| serialize_item(item) }, + largest_obligation: largest_obligation(period) + }.compact + end + + def serialize_item(item) + { + name: item.occurrence.recurring_transaction.display_name, + due_on: item.occurrence.due_on.iso8601, + this_period_share: fmt(item.share), + whole_obligation_remaining: fmt(item.remaining_total) + } + end + + def largest_obligation(period) + item = period.largest_obligation + return nil if item.nil? + + { + name: item.occurrence.recurring_transaction.display_name, + remaining_total: fmt(item.remaining_total) + } + end + + # Planner sums over an empty side come back as bare zero, not Money. + def fmt(value) + value.respond_to?(:format) ? value.format : Money.new(value, family.currency).format + end +end diff --git a/app/models/assistant/function/record_bill_payment.rb b/app/models/assistant/function/record_bill_payment.rb new file mode 100644 index 000000000..71b42efad --- /dev/null +++ b/app/models/assistant/function/record_bill_payment.rb @@ -0,0 +1,184 @@ +class Assistant::Function::RecordBillPayment < Assistant::Function + include Assistant::Function::BillsSupport + + class << self + def name + "record_bill_payment" + end + + def description + <<~INSTRUCTIONS + Record a payment against a bill's open occurrence, or settle it in full. + + - Omit amount to settle the occurrence completely (the remainder is recorded + as a manual payment with no transaction attached). + - Pass amount for a partial payment; the occurrence stays open until payments + cover the expected amount. + - occurrence_due_on picks a specific open occurrence by its due date; omitted, + the current (earliest open) occurrence is used. + - Linking a payment to a specific bank transaction is not possible here: the + app's matching engine and its review queue own that, so suggest the user + confirms matches on the Bills page instead. + + Confirm with the user before recording anything. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: %w[bill_id], + properties: { + bill_id: { type: "string", description: "The bill's id, exactly as returned by get_bills." }, + occurrence_due_on: { type: "string", description: "Due date (YYYY-MM-DD) of the open occurrence to pay. Defaults to the current one." }, + amount: { type: "number", minimum: 0.01, description: "Partial payment magnitude. Omit to settle in full." }, + paid_on: { type: "string", description: "When it was paid, YYYY-MM-DD. Defaults to today." } + } + ) + end + + AMOUNT_HINT = "Check the occurrence's remaining amount via get_bill_details and retry once with a valid amount." + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + series, error = find_writable_series(params["bill_id"]) + return error if error + + # Settling means the amount was not provided at all. A present-but-blank + # amount is malformed input, not a request to settle: it falls through to + # parse_amount and is rejected there. + settling = params["amount"].nil? + + occurrence, occurrence_error = resolve_occurrence( + series, params["occurrence_due_on"], settling: settling + ) + return occurrence_error if occurrence_error + + paid_on = parse_paid_on(params["paid_on"]) + return paid_on if paid_on.is_a?(Hash) + + allocator = RecurringTransaction::Allocator.new(occurrence) + + if settling + allocator.mark_paid!(paid_on: paid_on) + else + amount = parse_amount(params["amount"]) + return amount if amount.is_a?(Hash) + + # Friendly pre-check only: the authoritative remainder guard runs inside + # Allocator#allocate! under the occurrence lock, where two concurrent + # calls cannot both read the same stale capacity. + capacity = check_capacity(occurrence, amount) + return capacity if capacity + + allocator.allocate!(amount: amount, paid_on: paid_on, source: "user_created", cap_at_remaining: true) + end + + { recorded: true, bill: series.display_name, occurrence: serialize_occurrence(occurrence.reload).merge(status: occurrence.status) } + rescue RecurringTransaction::Allocator::OverAllocationError, ActiveRecord::RecordInvalid, ArgumentError => e + { + error: e.message, + hint: "Check the occurrence's remaining amount via get_bill_details and retry once with a valid amount." + } + end + + private + def resolve_occurrence(series, due_on, settling: false) + if due_on.present? + date = begin + Date.parse(due_on.to_s) + rescue Date::Error + nil + end + + if date.nil? + return [ nil, { error: "occurrence_due_on is not a valid date", hint: "Use YYYY-MM-DD." } ] + end + + occurrence = series.recurring_occurrences.open_status.find_by(due_on: date) + return [ occurrence, nil ] if occurrence + + [ nil, { + error: "No open occurrence of #{series.display_name} is due on #{date.iso8601}", + hint: "Open due dates: #{open_due_dates(series)}. Retry once with one of them, or omit occurrence_due_on." + } ] + else + occurrence = series.current_occurrence + + # Settling without naming a cycle means the one that is owed now. After + # a cycle is paid the next one becomes current, so a retried settle used + # to fall straight through and close NEXT month too: two identical + # requests, two cycles paid, no money moved for either. + # + # Only the settle path is guarded. Sending an explicit amount toward the + # next open cycle is a deliberate act with its own test, and an amount + # is exactly what a blind retry of a settle does not carry. + if settling && occurrence&.scheduled? && occurrence.derived_state == :upcoming + return [ nil, { + error: "#{series.display_name} has nothing owed right now", + hint: "Its next cycle is due #{occurrence.due_on.iso8601} and is not owed yet. " \ + "If you really mean to pay ahead, retry with occurrence_due_on set to that date." + } ] + end + + return [ occurrence, nil ] if occurrence&.scheduled? + + [ nil, { + error: "#{series.display_name} has no open occurrence to pay", + hint: "Nothing is currently owed on this bill. Use get_bill_details to see its state." + } ] + end + end + + # BigDecimal parses "Infinity" and "NaN"; the tool caller does not enforce + # params_schema, so both are rejected here rather than escaping as a raw + # PG::NumericValueOutOfRange. A negative was previously flipped with .abs, + # which turned a confused model into a silent payment. + def parse_amount(value) + magnitude = begin + BigDecimal(value.to_s) + rescue ArgumentError + nil + end + + return { error: "amount is not a number", hint: AMOUNT_HINT } if magnitude.nil? || !magnitude.finite? + return { error: "amount must be greater than zero", hint: AMOUNT_HINT } unless magnitude.positive? + + magnitude + end + + # An occurrence cannot absorb more than it owes. The allocator only ever + # capped payments attached to a bank entry, so every payment recorded + # through this tool was capped by nothing: two identical $1,500 retries + # settled a $2,000 bill at $3,000, and $999,999 was accepted outright. The + # error hint has always promised this ceiling; now it exists. + def check_capacity(occurrence, amount) + remaining = occurrence.remaining_amount + return nil if amount <= remaining + + { + error: "#{Money.new(amount, occurrence.currency).format} is more than the " \ + "#{occurrence.remaining_amount_money.format} still owed on the cycle due " \ + "#{occurrence.due_on.iso8601}", + hint: "Record at most the remaining amount. If this payment was already recorded, do not retry." + } + end + + def open_due_dates(series) + dates = series.recurring_occurrences.open_status.order(:due_on).limit(6).pluck(:due_on) + dates.any? ? dates.map(&:iso8601).join(", ") : "none" + end + + def parse_paid_on(value) + return Date.current if value.blank? + + Date.parse(value.to_s) + rescue Date::Error + { error: "paid_on is not a valid date", hint: "Use YYYY-MM-DD." } + end +end diff --git a/app/models/assistant/function/update_bill.rb b/app/models/assistant/function/update_bill.rb new file mode 100644 index 000000000..26a2c8892 --- /dev/null +++ b/app/models/assistant/function/update_bill.rb @@ -0,0 +1,274 @@ +class Assistant::Function::UpdateBill < Assistant::Function + include Assistant::Function::BillsSupport + + # UI-parity only: exactly the fields the edit dialog exposes. No income or + # transfer flips (they change sign and shape semantics), no end-condition + # juggling, no currency. + EDITABLE_BILL_TYPES = %w[bill subscription installment].freeze + + class << self + def name + "update_bill" + end + + def description + <<~INSTRUCTIONS + Update one bill's configuration. Only pass the fields being changed. + + Rules: + - amount is a positive magnitude; the bill keeps its own direction. An amount + change applies from today forward -- occurrences already due keep the old + figure, so a rent raise never restates last month's unpaid rent. + - account_name / category_name must exactly match names from get_accounts / + get_categories. Pass category_name "Uncategorized" to clear the category. + - Changing the schedule (frequency / due day) pins it: automatic detection + will respect it as the user's intent from then on. + - status "paused" sets the bill aside (no new occurrences); "active" resumes it. + - bill_type can move between bill, subscription and installment only. + + Confirm the change with the user before calling this. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: %w[bill_id], + properties: { + bill_id: { type: "string", description: "The bill's id, exactly as returned by get_bills." }, + name: { type: "string" }, + amount: { type: "number", minimum: 0.01, description: "New positive magnitude; applies forward only." }, + account_name: { type: "string", description: "Exact account name from get_accounts." }, + category_name: { type: "string", description: "Exact category name, or \"Uncategorized\" to clear." }, + bill_type: { type: "string", enum: EDITABLE_BILL_TYPES }, + status: { type: "string", enum: %w[active paused] }, + frequency: { + type: "string", + enum: RecurringTransaction::FrequencyPreset::PRESETS, + description: "New cadence. Combine with due_day_of_month / weekday / month_of_year as the cadence needs." + }, + due_day_of_month: { type: "integer", minimum: 1, maximum: 31 }, + weekday: { type: "integer", minimum: 0, maximum: 6, description: "0 = Sunday." }, + month_of_year: { type: "integer", minimum: 1, maximum: 12 }, + autopay: { type: "boolean" }, + payment_url: { type: "string" }, + notes: { type: "string" }, + renews_on: { type: "string", description: "YYYY-MM-DD" }, + trial_ends_on: { type: "string", description: "YYYY-MM-DD" } + } + ) + end + + def call(params = {}) + return recurring_disabled_result if recurring_disabled? + + series, error = find_writable_series(params["bill_id"]) + return error if error + + changed = [] + + # Each step returns an error hash or nil; the first error aborts the call + # before anything is saved. + %i[apply_simple_fields apply_bill_type apply_amount apply_account + apply_category apply_status apply_schedule].each do |step| + if (error = send(step, series, params, changed)) + return error + end + end + + if changed.empty? + return { error: "No recognized fields to change", hint: "Pass at least one editable field." } + end + + unless series.save + return { error: series.errors.full_messages.to_sentence, hint: "Fix the named fields and retry once." } + end + + { updated: true, changed_fields: changed, bill: serialize_series(series.reload) } + end + + private + def apply_simple_fields(series, params, changed) + { "name" => :name, "autopay" => :autopay, "payment_url" => :payment_url, + "notes" => :notes, "renews_on" => :renews_on, "trial_ends_on" => :trial_ends_on }.each do |key, attribute| + next unless params.key?(key) + + value = params[key] + + # AR silently casts an unparseable date string to nil, which would + # read back as "cleared" instead of "rejected". A blank still clears. + if %w[renews_on trial_ends_on].include?(key) && value.present? + begin + value = Date.parse(value.to_s) + rescue Date::Error + return { error: "#{key} is not a valid date", hint: "Use YYYY-MM-DD." } + end + end + + series.public_send("#{attribute}=", value) + changed << key + end + nil + end + + def apply_bill_type(series, params, changed) + return nil unless params.key?("bill_type") + + unless EDITABLE_BILL_TYPES.include?(series.bill_type) + return { + error: "#{series.display_name} is #{series.bill_type} and cannot change kind here", + hint: "Only bill, subscription and installment kinds are editable." + } + end + + unless params["bill_type"].presence_in(EDITABLE_BILL_TYPES) + return { + error: "#{params["bill_type"].inspect} is not a kind this tool can set", + hint: "Use one of: #{EDITABLE_BILL_TYPES.join(', ')}." + } + end + + series.bill_type = params["bill_type"] + changed << "bill_type" + nil + end + + # Magnitude in, sign preserved: income is stored negative, so a raw + # assignment would flip a paycheck into a bill. Forward-only pinning of + # already-due occurrences is the model's own callback. + def apply_amount(series, params, changed) + return nil unless params.key?("amount") + + # BigDecimal parses "Infinity" and "NaN" as non-finite numbers; the tool + # caller does not enforce params_schema, so both are rejected here the + # same as unparseable input. + magnitude = begin + BigDecimal(params["amount"].to_s).abs + rescue ArgumentError + nil + end + if magnitude.nil? || !magnitude.finite? + return { error: "amount is not a number", hint: "Pass a positive numeric magnitude." } + end + if magnitude.zero? + return { error: "amount must be greater than zero", hint: "Pass a positive magnitude." } + end + + series.amount = series.typed_income? ? -magnitude : magnitude + changed << "amount" + nil + end + + # Writable, not merely visible: attaching a series to an account changes + # what that account's owners see, so a read-only share is not a + # destination (same contract as the edit dialog's account resolution). + def apply_account(series, params, changed) + return nil unless params.key?("account_name") + + matches = Account.writable_by(user).where(name: params["account_name"]).limit(2).to_a + if matches.empty? + return { + error: "No account named #{params["account_name"].inspect} that you can add bills to", + hint: "Call get_accounts and retry once with the exact name of a writable account." + } + end + if matches.size > 1 + return { + error: "More than one account is named #{params["account_name"].inspect}", + hint: "Ask the user which one they mean; this tool cannot pick between namesakes." + } + end + + series.account = matches.first + changed << "account" + nil + end + + def apply_category(series, params, changed) + return nil unless params.key?("category_name") + + name = params["category_name"] + if name == "Uncategorized" + series.category_id = nil + changed << "category" + return nil + end + + # Category namesakes cannot exist: names are unique per family + # (index_categories_on_family_id_and_name), so find_by is unambiguous. + category = family.categories.find_by(name: name) + if category.nil? + return { + error: "No category named #{name.inspect}", + hint: "Call get_categories and retry once with the exact category name." + } + end + + series.category = category + changed << "category" + nil + end + + # The stored value the UI's own Pause writes is "inactive"; "paused" is + # the user-facing word for that state. + def apply_status(series, params, changed) + return nil unless params.key?("status") + + case params["status"] + when "active" then series.status = "active" + when "paused" then series.status = "inactive" + else + return { + error: "#{params["status"].inspect} is not a status this tool can set", + hint: "Use active or paused." + } + end + changed << "status" + nil + end + + # An AI-applied cadence is user intent by proxy: pin it so detection + # cannot quietly move it back, exactly as the edit dialog does. + def apply_schedule(series, params, changed) + companions = %w[due_day_of_month weekday month_of_year].select { |key| params.key?(key) } + + unless params.key?("frequency") + # Day details without a cadence would be silently dropped; a financial + # write is the wrong place to guess which cadence they belong to. + if companions.any? + return { + error: "#{companions.to_sentence} need frequency alongside them", + hint: "Pass frequency too (get_bill_details shows the current one)." + } + end + return nil + end + + # Same contract as create_bill: an unrecognized cadence is rejected, not + # guessed around, and the other edits in this call are not saved with it. + unless params["frequency"].to_s.presence_in(RecurringTransaction::FrequencyPreset::PRESETS) + return { + error: "#{params["frequency"]} is not a frequency this app recognizes", + hint: "Use one of: #{RecurringTransaction::FrequencyPreset::PRESETS.join(', ')}." + } + end + + applied = RecurringTransaction::FrequencyPreset.apply( + series, + preset: params["frequency"], + day_of_month: params["due_day_of_month"], + weekday: params["weekday"], + month_of_year: params["month_of_year"] + ) + + if applied + series.pin_schedule + changed << "schedule" + end + nil + end +end diff --git a/app/models/assistant/function_tool_caller.rb b/app/models/assistant/function_tool_caller.rb index 3faaa5cfd..487a3da43 100644 --- a/app/models/assistant/function_tool_caller.rb +++ b/app/models/assistant/function_tool_caller.rb @@ -36,17 +36,29 @@ class Assistant::FunctionToolCaller fn_args = JSON.parse(function_request.function_args.presence || "{}") fn.call(fn_args) - rescue JSON::ParserError + rescue JSON::ParserError => e + Rails.logger.warn("Assistant tool #{function_request.function_name} got invalid JSON arguments: #{e.class}: #{e.message}") + { error: "Arguments were not valid JSON", hint: "Re-send #{function_request.function_name} with valid JSON arguments." } rescue ActiveRecord::RecordNotFound => e + Rails.logger.warn("Assistant tool #{function_request.function_name} raised #{e.class}: #{e.message}") + + # The raised message carries the scoped relation's full SQL, so returning + # it verbatim handed any caller the access-control schema: the tables, the + # owner/share join, the lot. MCP passes this straight through to an + # external client, which needs only a guessed UUID to read it. The message + # says nothing the caller can act on that the hint does not, and a + # not-found is deliberately indistinguishable from a forbidden id. { - error: e.message, + error: "No such record, or it is not one you have access to", 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 + Rails.logger.warn("Assistant tool #{function_request.function_name} raised #{e.class}: #{e.message}") + { error: e.message, hint: "Check argument formats (dates are YYYY-MM-DD) and retry once with corrected arguments." diff --git a/app/models/recurring_transaction/allocator.rb b/app/models/recurring_transaction/allocator.rb index cb2ad58db..add4cb2a0 100644 --- a/app/models/recurring_transaction/allocator.rb +++ b/app/models/recurring_transaction/allocator.rb @@ -32,13 +32,25 @@ class RecurringTransaction # entry-less manual payment. Amounts are in the occurrence's currency; # a cross-currency entry converts at its own date's rate, or requires an # explicit amount when no rate exists. - def allocate!(amount: nil, entry: nil, paid_on: nil, source: nil) + def allocate!(amount: nil, entry: nil, paid_on: nil, source: nil, cap_at_remaining: false) occurrence.with_lock do with_entry_lock(entry) do allocated, source_amount, source_currency = resolve_amounts(amount, entry) guard_entry_capacity!(entry, source_amount) if entry freeze_expected_amount! + # Opt-in, because exceeding the remainder is load-bearing elsewhere: + # a single settlement above the expected amount is exactly how a + # price rise gets recorded and learned from. A caller that promises + # capping (the AI payment tool) enforces it HERE, under the + # occurrence lock, because its own pre-lock check is advisory only: + # two concurrent payments can both read the same stale remainder. + if cap_at_remaining && allocated > occurrence.remaining_amount + raise OverAllocationError, I18n.t("recurring_transactions.allocator.exceeds_remaining", + amount: Money.new(allocated, occurrence.currency).format, + remaining: Money.new(occurrence.remaining_amount, occurrence.currency).format) + end + allocation = occurrence.allocations.create!( entry: entry, allocated_amount: allocated, diff --git a/config/locales/models/recurring_transaction/en.yml b/config/locales/models/recurring_transaction/en.yml index 3b686f320..70768adc2 100644 --- a/config/locales/models/recurring_transaction/en.yml +++ b/config/locales/models/recurring_transaction/en.yml @@ -5,6 +5,7 @@ en: missing_rate: "no exchange rate from %{from} to %{to}; enter an amount explicitly" unmeasurable: "this transaction carries an allocation that cannot be converted to %{currency}, so what is left of it cannot be measured. Add an exchange rate for %{date} or remove the existing payment first" over_allocated: "allocating %{amount} exceeds the transaction's remaining %{capacity} %{currency}" + exceeds_remaining: "%{amount} exceeds the %{remaining} remaining on this cycle" activerecord: attributes: recurring_transaction: diff --git a/docs/hosting/mcp.md b/docs/hosting/mcp.md index e22598dce..d223b4283 100644 --- a/docs/hosting/mcp.md +++ b/docs/hosting/mcp.md @@ -153,8 +153,6 @@ At the time of writing, `tools/list` includes: | `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 | -These are the same tools used by Sure's built-in AI assistant. - ### Preview Tools These additional tools appear only when the MCP user has opted into preview @@ -172,6 +170,20 @@ permissions enforced in the web UI. | `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 | +| `get_bills` | List bills, subscriptions and other recurring obligations with each one's current payment state | +| `get_bill_details` | One bill's full configuration, open occurrences, payment history, price-change history and cost analytics | +| `get_paycheck_plan` | Income plan sliced into pay periods: what is due before the next payday, what stays reserved for later bills, what is safe to spend | +| `get_bill_audit` | Deterministic bills review: possible duplicates, price changes, trials about to convert, upcoming renewals, long-overdue bills and undeclared recurring patterns | +| `create_bill` | Create a bill, subscription, installment plan or income schedule | +| `update_bill` | Update one bill's configuration; amount changes apply from today forward | +| `record_bill_payment` | Record a partial payment against a bill's open occurrence, or settle it in full | + +Because tool calls never pass through the Bills pages' controllers, the bills +tools re-check the family's recurring-transactions feature gate (Settings → +Recurring transactions) and the MCP user's per-account access on every call. +With the feature disabled they return an error result instead of data, bills +tied to accounts the user cannot see are never returned, and the write tools +refuse series on accounts shared with the user read-only. They exist for agents that maintain a document-backed record of a family's wealth over time. See diff --git a/test/controllers/mcp_controller_test.rb b/test/controllers/mcp_controller_test.rb index bc4eb7cb7..72b94747a 100644 --- a/test/controllers/mcp_controller_test.rb +++ b/test/controllers/mcp_controller_test.rb @@ -585,7 +585,14 @@ class McpControllerTest < ActionDispatch::IntegrationTest assert result["isError"], "Expected isError to be true" inner = JSON.parse(result["content"][0]["text"]) - assert_equal "test error", inner["error"] + + # The raised text is written for a log, not for an external client: a + # RecordNotFound carries the access-control SQL and a range error carries + # the column definition. The client learns which tool failed; the detail + # stays server-side. + assert_equal "The tool failed to run", inner["error"] + assert_equal "get_balance_sheet", inner["tool"] + assert_no_match(/test error/, response.body) end end diff --git a/test/models/assistant/configurable_test.rb b/test/models/assistant/configurable_test.rb index c686e9604..a10c97dc5 100644 --- a/test/models/assistant/configurable_test.rb +++ b/test/models/assistant/configurable_test.rb @@ -78,4 +78,21 @@ class AssistantConfigurableTest < ActiveSupport::TestCase assert_match(/\d+ accounts:/, instructions) assert_match(/\d+ categories\./, instructions) end + # The tool caller returns {error:, hint:} instead of raising; without this + # rule the model sees those results as opaque data and never self-corrects. + test "instructions teach the model to follow tool error hints" do + config = Assistant.config_for(chats(:one)) + + assert_includes config[:instructions], + %(If a tool result contains an "error" and a "hint", follow the hint and retry once) + end + + # Function names are plumbing; a reply that says "I ran get_bill_audit" + # reads like a stack trace, not an assistant. + test "instructions forbid naming internal tools in responses" do + config = Assistant.config_for(chats(:one)) + + assert_includes config[:instructions], + "Never mention internal tool or function names in your responses" + end end diff --git a/test/models/assistant/function/bills_tools_schema_test.rb b/test/models/assistant/function/bills_tools_schema_test.rb new file mode 100644 index 000000000..5563b8a02 --- /dev/null +++ b/test/models/assistant/function/bills_tools_schema_test.rb @@ -0,0 +1,74 @@ +require "test_helper" + +class Assistant::Function::BillsToolsSchemaTest < ActiveSupport::TestCase + BILLS_TOOLS = [ + Assistant::Function::GetBills, + Assistant::Function::GetBillDetails, + Assistant::Function::GetPaycheckPlan, + Assistant::Function::GetBillAudit, + Assistant::Function::CreateBill, + Assistant::Function::UpdateBill, + Assistant::Function::RecordBillPayment + ].freeze + + # Strict function calling requires every declared property in `required`; + # a strict tool with an optional property is rejected wholesale by strict + # providers. Scoped to the bills tools: the pre-existing tools' strictness + # is reworked by the assistant-upgrade PR (#3064), not here. + test "strict bills tools declare every property as required" do + each_definition do |name, 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, + "#{name} is strict but properties #{property_keys - required_keys} are not required" + end + end + + # Data-driven enums put per-family values into the schema, which breaks + # strict validators the moment a family has none (the empty-enum incident) + # and bloats every request. Bills tools use static enums only. + test "bills tool enums are static and never empty" do + each_definition do |name, definition| + definition[:params_schema][:properties].each do |key, property| + enums = [ property[:enum], property.dig(:items, :enum) ].compact + enums.each do |values| + assert values.any?, "#{name}.#{key} declares an empty enum" + end + end + end + end + + # The registry is shared with the public /mcp endpoint: being listed makes a + # tool callable by external agents. Bills is a preview feature, so its tools + # ride the per-user preview flag -- present for an opted-in user, absent from + # the default set. This test documents both halves. + test "the bills tools are preview-gated in the registry" do + user_with_preview = users(:family_admin) + user_with_preview.update!( + preferences: (user_with_preview.preferences || {}).merge("preview_features_enabled" => true) + ) + + preview_classes = Assistant.function_classes(user_with_preview) + default_classes = Assistant.function_classes(nil) + + BILLS_TOOLS.each do |tool| + assert_includes preview_classes, tool + assert_not_includes default_classes, tool + end + end + + private + + def each_definition + user = users(:family_admin) + + BILLS_TOOLS.each do |fn_class| + fn = fn_class.new(user) + yield fn.name, fn.to_definition + end + end +end diff --git a/test/models/assistant/function/create_bill_test.rb b/test/models/assistant/function/create_bill_test.rb new file mode 100644 index 000000000..d7e420b50 --- /dev/null +++ b/test/models/assistant/function/create_bill_test.rb @@ -0,0 +1,170 @@ +require "test_helper" + +class Assistant::Function::CreateBillTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + end + + test "creates a declared bill with schedule and upcoming dates" do + due = Date.current.beginning_of_month.next_month + 8.days + + result = call_tool( + "name" => "City Water", "amount" => 80, "first_due_on" => due.iso8601, + "account_name" => accounts(:depository).name, "bill_type" => "subscription" + ) + + assert result[:created] + assert_equal "City Water", result[:bill][:name] + assert_equal "subscription", result[:bill][:bill_type] + assert_equal 3, result[:upcoming_due_dates].size + + series = @family.recurring_transactions.find_by!(name: "City Water") + assert series.manual?, "an AI-created bill is a declared bill" + assert_equal accounts(:depository).id, series.account_id + assert_operator series.recurring_occurrences.count, :>, 0 + end + + test "income flips the stored sign, never the caller's" do + result = call_tool( + "name" => "Paycheck", "amount" => 1200, + "first_due_on" => (Date.current + 3).iso8601, "is_income" => true + ) + + assert result[:created] + series = @family.recurring_transactions.find_by!(name: "Paycheck") + assert series.amount.negative?, "income is stored negative" + assert_equal "income", series.bill_type + end + + test "an unknown account name returns a hint instead of guessing" do + result = call_tool( + "name" => "Bill", "amount" => 10, "first_due_on" => Date.current.iso8601, + "account_name" => "No Such Account" + ) + + assert_match(/No account named/, result[:error]) + assert_includes result[:hint], "get_accounts" + assert_equal 0, @family.recurring_transactions.count + end + + test "another family's category can never be attached" do + foreign = families(:empty).categories.create!(name: "Foreign category", color: "#ff0000") + + result = call_tool( + "name" => "Bill", "amount" => 10, "first_due_on" => Date.current.iso8601, + "category_name" => foreign.name + ) + + assert_match(/No category named/, result[:error], + "a category outside the family must resolve to nothing") + assert_equal 0, @family.recurring_transactions.count + end + + test "a duplicate identity lands as a second tier via the dedup retry" do + 2.times do |i| + result = call_tool( + "name" => "Streaming Co", "amount" => 15.99 + i, + "first_due_on" => Date.current.iso8601, + "account_name" => accounts(:depository).name + ) + assert result[:created], "attempt #{i + 1} must save" + end + + assert_equal 2, @family.recurring_transactions.where(name: "Streaming Co").count + end + + test "an invalid date returns the validation message as an error" do + result = call_tool("name" => "Bill", "amount" => 10, "first_due_on" => "not-a-date") + + assert result[:error].present? + assert result[:hint].present? + end + + test "a non-numeric amount returns an error instead of raising" do + result = call_tool("name" => "Bill", "amount" => "abc", "first_due_on" => (Date.current + 5).iso8601) + + assert result[:error].present? + assert result[:hint].present? + assert_not @family.recurring_transactions.exists?(name: "Bill") + end + + + # Every dedup index is keyed on account_id, and Postgres treats NULLs as + # distinct, so an account-less bill collided with nothing and an LLM retry + # doubled the family's recurring commitments. + test "an account-less bill is not duplicated by a retry" do + args = { "name" => "Netflix", "amount" => 15.99, "first_due_on" => (Date.current + 3).iso8601 } + + first = call_tool(args) + second = call_tool(args) + + assert first[:created] + assert second[:error].present?, "the retry must be recognized as the same bill" + assert_equal 1, @family.recurring_transactions.where(name: "Netflix").count + end + + test "a different amount is still a distinct account-less bill" do + call_tool("name" => "Netflix", "amount" => 15.99, "first_due_on" => (Date.current + 3).iso8601) + second = call_tool("name" => "Netflix", "amount" => 24.99, "first_due_on" => (Date.current + 3).iso8601) + + assert second[:created], "a price tier is a real second row, not a duplicate" + assert_equal 2, @family.recurring_transactions.where(name: "Netflix").count + end + + # "annual" is the enum word; "yearly" is the equally natural thing a model + # says. It used to fall back to monthly, turning a $600 premium into a $600 + # monthly obligation. + test "an unrecognized frequency is refused rather than made monthly" do + result = call_tool("name" => "Insurance", "amount" => 600, "first_due_on" => (Date.current + 3).iso8601, "frequency" => "yearly") + + assert result[:error].present? + assert_match(/not a frequency/, result[:error]) + assert_match(/annual/, result[:hint], "the hint has to name the word that works") + assert_equal 0, @family.recurring_transactions.where(name: "Insurance").count + end + + test "a recognized frequency still applies" do + result = call_tool("name" => "Insurance", "amount" => 600, "first_due_on" => (Date.current + 3).iso8601, "frequency" => "annual") + + assert result[:created] + series = @family.recurring_transactions.find_by(name: "Insurance") + assert_equal "annual", RecurringTransaction::FrequencyPreset.detect(series).key + end + + # The tool caller does not enforce params_schema: a loose MCP client can + # send booleans as strings, and "true" == true is false in Ruby, which used + # to silently declare a paycheck as a bill. + test "boolean-ish strings cast and garbage booleans are refused" do + result = call_tool("name" => "Paycheck", "amount" => 1200, + "first_due_on" => (Date.current + 3).iso8601, "is_income" => "true") + + assert result[:created] + assert @family.recurring_transactions.find_by(name: "Paycheck").amount.negative?, + "a string true must still declare income, stored negative" + + garbage = call_tool("name" => "Maybe", "amount" => 10, + "first_due_on" => (Date.current + 3).iso8601, "is_income" => "yeah") + assert_match(/must be true or false/, garbage[:error]) + assert_nil @family.recurring_transactions.find_by(name: "Maybe") + end + + test "a read-only shared account is not a creation destination" do + member = users(:family_member) + + result = Assistant::Function::CreateBill.new(member).call( + "name" => "Sneaky", "amount" => 10, "first_due_on" => (Date.current + 3).iso8601, + "account_name" => accounts(:credit_card).name + ) + + assert_match(/add bills to/, result[:error]) + assert_nil @family.recurring_transactions.find_by(name: "Sneaky") + end + + private + + def call_tool(params) + Assistant::Function::CreateBill.new(@user).call(params) + end +end diff --git a/test/models/assistant/function/get_bill_audit_test.rb b/test/models/assistant/function/get_bill_audit_test.rb new file mode 100644 index 000000000..65657eb9a --- /dev/null +++ b/test/models/assistant/function/get_bill_audit_test.rb @@ -0,0 +1,133 @@ +require "test_helper" + +class Assistant::Function::GetBillAuditTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + end + + test "flags exact duplicates but never subscription tiers" do + create_series(name: "Streaming Co", amount: 15.99, dedup_scope: "a") + create_series(name: "Streaming Co", amount: 15.99, dedup_scope: "b") + # A different tier to the same merchant: same name, different amount. + create_series(name: "Streaming Co", amount: 24.99, dedup_scope: "c") + + result = call_tool + + duplicates = result[:possible_duplicates][:items] + assert_equal 1, duplicates.size + assert_equal 2, duplicates.sole[:bills].size, + "the 24.99 tier must not be flagged as a duplicate of the 15.99 pair" + end + + test "reports price changes inside the lookback and trials about to convert" do + series = create_series(name: "Stream Co", amount: 12) + series.recurring_price_changes.create!( + effective_on: Date.current - 20, previous_amount: 10, new_amount: 12, + currency: "USD", source: "detected" + ) + create_series(name: "Trial service", amount: 9, trial_ends_on: Date.current + 5) + + result = call_tool + + assert_equal 1, result[:price_changes][:count] + assert_in_delta 20.0, result[:price_changes][:items].sole[:percent_change] + assert_equal "Trial service", result[:upcoming_trials][:items].sole[:name] + end + + test "suggested detections wait in awaiting_confirmation" do + create_series(name: "Detected sub", amount: 8, status: "suggested", manual: false) + + result = call_tool + + assert_equal 1, result[:awaiting_confirmation][:count] + assert_equal "Detected sub", result[:awaiting_confirmation][:items].sole[:name] + end + + test "surfaces recurring charge patterns no series covers" do + 3.times do |i| + accounts(:depository).entries.create!( + date: Date.current - i.months, amount: 40, currency: "USD", + name: "GYM MEMBERSHIP", entryable: Transaction.new + ) + end + + result = call_tool + + candidate = result[:undeclared_candidates][:items].find { |item| item[:name] == "GYM MEMBERSHIP" } + assert candidate.present?, "the undeclared gym pattern must surface" + assert_operator candidate[:occurrence_count], :>=, 2 + end + + test "undeclared candidates never include accounts the user cannot reach" do + 3.times do |i| + accounts(:investment).entries.create!( + date: Date.current - i.months, amount: 25, currency: "USD", + name: "BROKERAGE FEE", entryable: Transaction.new + ) + end + + admin_names = call_tool[:undeclared_candidates][:items].map { |item| item[:name] } + assert_includes admin_names, "BROKERAGE FEE", + "positive control: the admin can see the investment-account pattern" + + member_result = Assistant::Function::GetBillAudit.new(users(:family_member)).call + member_names = member_result[:undeclared_candidates][:items].map { |item| item[:name] } + assert_not_includes member_names, "BROKERAGE FEE", + "a pattern on an account the member was never given must not leak" + end + + test "long_overdue measures in the bill's own cycles" do + overdue_day = 45.days.ago.to_date + series = create_series(name: "Forgotten bill", amount: 60, + expected_day_of_month: overdue_day.day, + last_occurrence_date: overdue_day << 1, + next_expected_date: overdue_day) + + # The cycle actually left unpaid is what makes a bill overdue. Declared + # series do not fabricate past occurrences, so the one that was forgotten + # has to exist for there to be anything to forget. + series.recurring_occurrences.destroy_all + series.recurring_occurrences.create!( + family: @family, original_due_on: overdue_day, due_on: overdue_day, + currency: "USD", expected_amount: 60, status: "scheduled" + ) + + result = call_tool + + row = result[:long_overdue][:items].find { |item| item[:name] == "Forgotten bill" } + assert row.present? + assert_operator row[:cycles_overdue], :>=, 1 + end + + # Same contract as get_bills' due_within_days: out-of-range values are + # rejected, not silently clamped onto a different question. + test "an out-of-range lookback_months is rejected, not silently adjusted" do + assert_match(/between 1 and 24/, call_tool("lookback_months" => 0)[:error]) + assert_match(/between 1 and 24/, call_tool("lookback_months" => "forever")[:error]) + assert_match(/between 1 and 24/, call_tool("lookback_months" => 36)[:error]) + assert_nil call_tool("lookback_months" => 6)[:error] + assert_nil call_tool({})[:error] + end + + private + + def call_tool(params = {}) + Assistant::Function::GetBillAudit.new(@user).call(params) + end + + def create_series(name:, amount:, dedup_scope: nil, **overrides) + @family.recurring_transactions.create!({ + name: name, + account: accounts(:depository), + amount: amount, + dedup_scope: dedup_scope || "#{name}-#{amount}", + currency: "USD", + expected_day_of_month: Date.current.day, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: Date.current, + status: "active" + }.merge(overrides)) + end +end diff --git a/test/models/assistant/function/get_bill_details_test.rb b/test/models/assistant/function/get_bill_details_test.rb new file mode 100644 index 000000000..20db358bc --- /dev/null +++ b/test/models/assistant/function/get_bill_details_test.rb @@ -0,0 +1,146 @@ +require "test_helper" + +class Assistant::Function::GetBillDetailsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + end + + test "analytics come from confirmed payments on settled occurrences, never estimates" do + series = create_series(name: "CITY WATER", amount: 200) + paid = series.recurring_occurrences.order(:due_on).first + paid.allocations.create!(allocated_amount: 100, currency: "USD", source: "user_created") + paid.close!("paid", source: "user") + # An open occurrence still expecting the estimated 200 must not move the average. + + result = call_tool(series.id) + + assert_equal "$100.00", result[:analytics][:average_paid], + "the open occurrence's 200 estimate must not be averaged in" + assert_equal "$100.00", result[:analytics][:paid_this_year] + end + + test "analytics are null before anything was paid" do + series = create_series(name: "Fresh bill", amount: 50) + + result = call_tool(series.id) + + assert_nil result[:analytics] + end + + test "history rows carry their payments with the transaction name" do + series = create_series(name: "CITY WATER", amount: 80) + entry = accounts(:depository).entries.create!( + date: Date.current, amount: 80, currency: "USD", name: "CITY WATER PMT", + entryable: Transaction.new + ) + occurrence = series.recurring_occurrences.order(:due_on).first + occurrence.allocations.create!(entry: entry, allocated_amount: 80, currency: "USD", source: "user_confirmed") + occurrence.close!("paid", source: "user") + + result = call_tool(series.id) + + payment = result[:history].sole[:payments].sole + assert_equal "CITY WATER PMT", payment[:transaction_name] + assert_equal "user_confirmed", payment[:source] + end + + test "price changes serialize with the percent" do + series = create_series(name: "Stream Co", amount: 12) + series.recurring_price_changes.create!( + effective_on: Date.current - 10, previous_amount: 10, new_amount: 12, + currency: "USD", source: "detected" + ) + + result = call_tool(series.id) + + change = result[:price_changes].sole + assert_equal "$10.00", change[:previous_amount] + assert_equal "$12.00", change[:new_amount] + assert_in_delta 20.0, change[:percent_change] + end + + test "an invalid uuid returns an error with a hint" do + result = call_tool("not-a-uuid") + + assert result[:error].present? + assert_includes result[:hint], "get_bills" + end + + test "another family's bill raises RecordNotFound for the caller to convert" do + other = families(:empty).recurring_transactions.create!( + name: "Foreign bill", amount: 10, currency: "USD", + expected_day_of_month: 1, last_occurrence_date: Date.current, + next_expected_date: 1.month.from_now.to_date, status: "active" + ) + + assert_raises(ActiveRecord::RecordNotFound) { call_tool(other.id) } + end + + + # annualized_cost was the declared amount times cadence, sitting inside a + # block documented as payments-only, so a bill declared at 100 whose every + # payment was 50 reported a 50 average and a 1,200 year in the same hash. + test "the run rate follows the payments, and the declared figure says so" do + series = paid_series(declared: 100, paid: 50, cycles: 3) + + analytics = call_tool(series.id)[:analytics] + + assert_equal "$50.00", analytics[:average_paid] + assert_equal "$600.00", analytics[:annualized_cost], "twelve payments of what is actually paid" + assert_equal "$1,200.00", analytics[:annualized_declared], "the declared figure keeps its own name" + end + + test "history discloses how much of it was withheld" do + series = paid_series(declared: 10, paid: 10, cycles: 15) + + result = call_tool(series.id) + + assert_equal 12, result[:history].size + assert_equal 15, result[:history_window][:count] + assert result[:history_window][:truncated], "summing 12 rows is not the lifetime total" + end + + private + + def call_tool(bill_id) + Assistant::Function::GetBillDetails.new(@user).call({ "bill_id" => bill_id.to_s }) + end + + # A series whose declared amount and actual payments deliberately disagree, + # with `cycles` settled cycles behind it. + def paid_series(declared:, paid:, cycles:) + series = create_series(name: "Gym #{declared}-#{paid}-#{cycles}", amount: declared) + series.recurring_occurrences.destroy_all + + cycles.times do |index| + due = Date.current - ((index + 1) * 30) + occurrence = series.recurring_occurrences.create!( + family: @family, original_due_on: due, due_on: due, + # The series declares one figure; each cycle actually costs another, + # which is the whole point of separating run rate from declared. + currency: "USD", expected_amount: paid, status: "scheduled" + ) + RecurringTransaction::Allocator.new(occurrence).allocate!( + amount: paid, paid_on: due, source: "user_created" + ) + end + + series.reload + end + + def create_series(name:, amount:, **overrides) + @family.recurring_transactions.create!({ + name: name, + account: accounts(:depository), + amount: amount, + dedup_scope: "#{name}-#{amount}", + currency: "USD", + expected_day_of_month: Date.current.day, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: Date.current, + status: "active" + }.merge(overrides)) + end +end diff --git a/test/models/assistant/function/get_bills_test.rb b/test/models/assistant/function/get_bills_test.rb new file mode 100644 index 000000000..dd957440b --- /dev/null +++ b/test/models/assistant/function/get_bills_test.rb @@ -0,0 +1,240 @@ +require "test_helper" + +class Assistant::Function::GetBillsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + end + + test "defaults to active bills and hides review states" do + create_series(name: "Active bill", amount: 50) + create_series(name: "Suggested detection", amount: 20, status: "suggested", manual: false) + create_series(name: "Dismissed", amount: 10, status: "ended") + create_series(name: "Paused bill", amount: 30, status: "inactive") + + result = call_tool + + names = result[:bills].map { |bill| bill[:name] } + assert_includes names, "Active bill" + assert_not_includes names, "Suggested detection" + assert_not_includes names, "Dismissed" + assert_not_includes names, "Paused bill" + end + + test "the paused filter speaks the UI vocabulary over the stored value" do + create_series(name: "Paused bill", amount: 30, status: "inactive") + + result = call_tool("status" => "paused") + + row = result[:bills].sole + assert_equal "Paused bill", row[:name] + assert_equal "paused", row[:status], "stored 'inactive' must serialize as the UI's word" + end + + test "payment_state filters on the current occurrence" do + travel_to Date.current do + overdue_day = 10.days.ago.to_date + create_series(name: "Late bill", amount: 75, + expected_day_of_month: overdue_day.day, + anchor_date: overdue_day, + last_occurrence_date: 2.months.ago.to_date, + next_expected_date: overdue_day) + # Anchored at its own future due date: the generator floors at anchor, + # so no past-cycle row exists to read as overdue. + future_day = Date.current + 10 + create_series(name: "Future bill", amount: 20, + expected_day_of_month: future_day.day, + anchor_date: future_day, + last_occurrence_date: future_day - 1.month, + next_expected_date: future_day) + + result = call_tool("payment_state" => "overdue") + + assert_equal [ "Late bill" ], result[:bills].map { |bill| bill[:name] } + end + end + + test "search matches the merchant behind a nameless series" do + create_series(name: nil, merchant: merchants(:netflix), amount: 15.99) + create_series(name: "Water", amount: 80) + + result = call_tool("search" => merchants(:netflix).name) + + assert_equal [ merchants(:netflix).name ], result[:bills].map { |bill| bill[:name] } + end + + test "a member only sees bills on accounts they were given" do + create_series(name: "Visible bill", amount: 10) + create_series(name: "Hidden brokerage bill", amount: 99, account: accounts(:investment)) + + result = Assistant::Function::GetBills.new(users(:family_member)).call({}) + + names = result[:bills].map { |bill| bill[:name] } + assert_includes names, "Visible bill" + assert_not_includes names, "Hidden brokerage bill" + end + + test "totals exclude income and transfers and count the overdue" do + create_series(name: "Real bill", amount: 100) + create_series(name: "Paycheck", amount: -2000, bill_type: "income") + create_series(name: "Card payment", amount: 300, bill_type: "transfer", + destination_account_id: accounts(:credit_card).id) + + result = call_tool("status" => "all") + + monthly = result[:totals][:active_monthly_equivalent_by_currency].fetch("USD") + assert_equal "$100.00", monthly, "income and transfers must not inflate the spend total" + end + + test "a disabled family gets an error with a hint, not a raise" do + @family.update!(recurring_transactions_disabled: true) + + result = call_tool + + assert_match(/disabled/, result[:error]) + assert result[:hint].present? + end + + # Reported from live use: "What am I paying monthly for subscriptions?" answered + # "No active subscriptions found, total monthly equivalent $0" while five + # detected subscriptions sat in `suggested`. The status filter was right; the + # silence about what it filtered out was the bug. + test "an empty status-filtered result says where the matches actually are" do + create_series(name: "Crunchyroll", amount: 12, status: "suggested", bill_type: "subscription", manual: false) + create_series(name: "Huntr", amount: 40, status: "suggested", bill_type: "subscription", manual: false) + + result = call_tool("bill_type" => "subscription") + + assert_equal 0, result[:total_results] + assert result[:hint].present?, "an empty result must say what other statuses hold" + assert_match(/2 suggested/, result[:hint]) + assert_match(/status: all/, result[:hint]) + end + + test "the hint counts only the requested bill type" do + create_series(name: "Crunchyroll", amount: 12, status: "suggested", bill_type: "subscription", manual: false) + create_series(name: "Rent", amount: 2000, status: "suggested", bill_type: "bill", manual: false) + + result = call_tool("bill_type" => "subscription") + + assert_equal 0, result[:total_results] + assert_match(/1 suggested/, result[:hint]) + end + + test "no hint when results are found" do + create_series(name: "Crunchyroll", amount: 12, bill_type: "subscription") + + result = call_tool("bill_type" => "subscription") + + assert_operator result[:total_results], :>, 0 + assert_nil result[:hint] + end + + test "no hint when the caller already asked for every status" do + create_series(name: "Rent", amount: 2000, status: "suggested", bill_type: "bill", manual: false) + + result = call_tool("status" => "all", "bill_type" => "subscription") + + assert_equal 0, result[:total_results] + assert_nil result[:hint], "status: all already saw everything, so there is nowhere else to point" + end + + + # strict_mode? is false and MCP bypasses provider validation, so out-of-schema + # values arrive routinely. Each one used to fail silently and differently. + test "an unknown bill_type is refused rather than dropping the filter" do + create_series(name: "Rent", amount: 2000) + create_series(name: "Netflix", amount: 12, bill_type: "subscription") + + result = call_tool("bill_type" => "subscriptions") + + assert_match(/not a valid bill_type/, result[:error], + "plural used to drop the filter and total rent as a subscription") + assert_nil result[:bills] + end + + test "an unknown status is refused rather than silently meaning active" do + create_series(name: "Rent", amount: 2000) + + result = call_tool("status" => "inactive") + + assert_match(/not a valid status/, result[:error]) + assert_match(/paused/, result[:hint], "the hint has to name the word that works") + end + + test "an unknown payment_state is refused rather than matching nothing" do + create_series(name: "Rent", amount: 2000) + + result = call_tool("payment_state" => "unpaid") + + assert_match(/not a valid payment_state/, result[:error]) + end + + test "valid filters still work" do + create_series(name: "Netflix", amount: 12, bill_type: "subscription") + + result = call_tool("bill_type" => "subscription") + + assert_equal 1, result[:total_results] + end + + # The hint can only redirect a status filter. Blaming status for a result + # emptied by payment_state sent the model to the suggestion queue while it + # was asking about overdue bills, and the retry it prescribed returned empty + # again with no hint at all. + test "the empty-result hint stays quiet when another filter did the emptying" do + create_series(name: "Rent", amount: 2000) + create_series(name: "Detected", amount: 9, status: "suggested", manual: false) + + result = call_tool("payment_state" => "overdue") + + assert_equal 0, result[:total_results] + assert_nil result[:hint], "nothing about the suggested series explains an overdue query" + end + + test "the hint still fires when status alone emptied the result" do + create_series(name: "Detected", amount: 9, status: "suggested", manual: false) + + result = call_tool({}) + + assert_equal 0, result[:total_results] + assert_match(/suggested/, result[:hint]) + end + + # A filter that silently answers a different question than it was asked is + # worse than an error: due_within_days: 0 used to clamp to 1. + test "an out-of-range due_within_days is rejected, not silently adjusted" do + create_series(name: "Rent", amount: 2150) + + assert_match(/between 1 and 365/, call_tool("due_within_days" => 0)[:error]) + assert_match(/between 1 and 365/, call_tool("due_within_days" => "soon")[:error]) + assert_match(/between 1 and 365/, call_tool("due_within_days" => 400)[:error]) + assert_nil call_tool("due_within_days" => 30)[:error] + end + + private + + def call_tool(params = {}) + Assistant::Function::GetBills.new(@user).call(params) + end + + def create_series(name:, amount:, merchant: nil, account: accounts(:depository), **overrides) + @family.recurring_transactions.create!({ + name: name, + merchant: merchant, + account: account, + amount: amount, + dedup_scope: "#{name}-#{amount}", + currency: "USD", + expected_day_of_month: Date.current.day, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: Date.current, + status: "active", + # The generator's anchor floor (no fabricated past debt) applies to + # declared series, which is what these test rows stand in for. + manual: true, + bill_type: amount.to_d.negative? ? "income" : "bill" + }.merge(overrides)) + end +end diff --git a/test/models/assistant/function/get_paycheck_plan_test.rb b/test/models/assistant/function/get_paycheck_plan_test.rb new file mode 100644 index 000000000..bbeba9956 --- /dev/null +++ b/test/models/assistant/function/get_paycheck_plan_test.rb @@ -0,0 +1,194 @@ +require "test_helper" + +class Assistant::Function::GetPaycheckPlanTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + end + + test "no declared income returns an error with a hint instead of a fabricated plan" do + # A detected inflow is income by sign but declares no payday. + @family.recurring_transactions.create!( + name: "Deposit From Checking", account: accounts(:depository), amount: -0.01, + currency: "USD", bill_type: "income", manual: false, + expected_day_of_month: Date.current.day, + last_occurrence_date: 1.month.ago.to_date, next_expected_date: Date.current, + status: "active" + ) + + result = call_tool + + assert_equal "No declared income schedule", result[:error] + assert_includes result[:hint], "declared" + end + + test "declared income produces periods with due, reserved and safe figures" do + payday = Date.current + 3 + @family.recurring_transactions.create!( + name: "Paycheck", account: accounts(:depository), amount: -1200, + currency: "USD", bill_type: "income", manual: true, + expected_day_of_month: payday.day, anchor_date: payday, + last_occurrence_date: payday, next_expected_date: payday, status: "active" + ) + due = payday + 4 + @family.recurring_transactions.create!( + name: "Rent", account: accounts(:depository), amount: 500, + currency: "USD", bill_type: "bill", manual: true, + expected_day_of_month: due.day, anchor_date: due, + last_occurrence_date: due, next_expected_date: due, status: "active" + ) + + result = call_tool("periods_limit" => 2) + + assert_equal @family.currency, result[:family_currency] + assert_operator result[:periods].size, :>=, 1 + + period_with_rent = result[:periods].find { |period| period[:bills_due].any? { |bill| bill[:name] == "Rent" } } + assert period_with_rent.present?, "the rent must land in a period as due" + assert period_with_rent[:income].present? + assert period_with_rent[:safe_after_bills].present? + end + + # A lone materialized paycheck landing today yields an empty plan; the tool + # answers with its guidance instead of raising on the missing periods. + test "a lone paycheck landing today returns guidance rather than raising" do + series = @family.recurring_transactions.create!( + name: "Paycheck", account: accounts(:depository), amount: -1840, + currency: "USD", bill_type: "income", manual: true, + expected_day_of_month: Date.current.day, anchor_date: Date.current, + last_occurrence_date: Date.current, next_expected_date: Date.current, status: "active" + ) + series.recurring_occurrences.where("due_on > ?", Date.current).delete_all + + result = call_tool + + assert_equal "No declared income schedule", result[:error] + assert result[:hint].present? + end + + # Same class of bug as the get_bills one: the planner counts confirmed series + # only, which is correct, but every figure it returns is spending headroom, so + # dropping unconfirmed detections without a word makes the plan look safer + # than it is. + test "a plan that ignores unconfirmed detections says so" do + declare_income + @family.recurring_transactions.create!( + name: "Detected subscription", account: accounts(:depository), amount: 40, + currency: "USD", expected_day_of_month: Date.current.day, status: "suggested", + bill_type: "subscription", manual: false, dedup_scope: "detected-40", + last_occurrence_date: 1.month.ago.to_date, next_expected_date: Date.current + ) + + result = call_tool + + assert result[:unconfirmed_excluded].present?, + "the plan must disclose obligations it left out" + assert_equal 1, result[:unconfirmed_excluded][:count] + assert_match(/upper bound/, result[:unconfirmed_excluded][:note]) + end + + test "no exclusion notice when everything is confirmed" do + declare_income + + result = call_tool + + assert result[:periods].present? + assert_nil result[:unconfirmed_excluded] + end + + + # The assistant answered "Shortfall: $150.00, Safe after bills: -$150.00" for + # a window funded entirely from money already in the bank. A bridge earns + # nothing, so income minus obligations is a deficit by construction, and the + # deficit sat right beside short: false. + test "a bridge window reports headroom against cash, not against its zero income" do + set_cash(900) + declare_future_income + declare_bridge_bill(amount: 150) + + bridge = call_tool[:periods].find { |period| period[:bridge] } + + assert_equal false, bridge[:short] + assert_equal "$750.00", bridge[:safe_after_bills], + "cash minus what is due out of it, never income minus obligations" + assert_equal "$900.00", bridge[:cash_on_hand] + end + + test "a bridge the cash cannot cover still reports short" do + set_cash(100) + declare_future_income + declare_bridge_bill(amount: 150) + + bridge = call_tool[:periods].find { |period| period[:bridge] } + + assert_equal true, bridge[:short] + assert_equal "-$50.00", bridge[:safe_after_bills] + end + + test "an unreadable balance omits the figure rather than inventing one" do + @family.accounts.update_all(status: "disabled") + declare_future_income + declare_bridge_bill(amount: 150) + + bridge = call_tool[:periods].find { |period| period[:bridge] } + + assert_equal false, bridge[:short] + assert_not bridge.key?(:safe_after_bills), + "no balance means no honest headroom figure, so the key goes rather than guessing" + assert_not bridge.key?(:cash_on_hand) + end + + private + + # The shared declare_income pays today, so there is no gap before the next + # payday and no bridge window at all. These tests need one. + def declare_future_income + payday = Date.current + 5 + series = @family.recurring_transactions.create!( + name: "Payday", account: accounts(:depository), amount: -2000, + currency: "USD", expected_day_of_month: payday.day, status: "active", + bill_type: "income", manual: true, dedup_scope: "future-payday--2000", + last_occurrence_date: 1.month.ago.to_date, next_expected_date: payday + ) + series.recurring_occurrences.destroy_all + series.recurring_occurrences.create!( + family: @family, original_due_on: payday, due_on: payday, + currency: "USD", expected_amount: 2000, status: "scheduled" + ) + series + end + + def set_cash(amount) + accounts = @family.accounts.where(accountable_type: "Depository") + accounts.update_all(balance: amount / accounts.count.to_d) + end + + def declare_bridge_bill(amount:) + series = @family.recurring_transactions.create!( + name: "Haircut", account: accounts(:depository), amount: amount, + currency: "USD", status: "active", bill_type: "bill", manual: true, + dedup_scope: "haircut-#{amount}", expected_day_of_month: (Date.current + 1).day, + last_occurrence_date: 1.month.ago.to_date, next_expected_date: Date.current + 1 + ) + series.recurring_occurrences.destroy_all + series.recurring_occurrences.create!( + family: @family, original_due_on: Date.current + 1, due_on: Date.current + 1, + currency: "USD", expected_amount: amount, status: "scheduled" + ) + series + end + + def call_tool(params = {}) + Assistant::Function::GetPaycheckPlan.new(@user).call(params) + end + + def declare_income + @family.recurring_transactions.create!( + name: "Payday", account: accounts(:depository), amount: -2000, + currency: "USD", expected_day_of_month: Date.current.day, status: "active", + bill_type: "income", manual: true, dedup_scope: "payday--2000", + last_occurrence_date: 1.month.ago.to_date, next_expected_date: Date.current + ) + end +end diff --git a/test/models/assistant/function/record_bill_payment_test.rb b/test/models/assistant/function/record_bill_payment_test.rb new file mode 100644 index 000000000..704c7ccaf --- /dev/null +++ b/test/models/assistant/function/record_bill_payment_test.rb @@ -0,0 +1,194 @@ +require "test_helper" + +class Assistant::Function::RecordBillPaymentTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + @series = @family.recurring_transactions.create!( + name: "Rent", account: accounts(:depository), amount: 2000, currency: "USD", + expected_day_of_month: Date.current.day, anchor_date: Date.current, + last_occurrence_date: 1.month.ago.to_date, next_expected_date: Date.current, + status: "active", manual: true + ) + @occurrence = @series.recurring_occurrences.open_status.order(:due_on).first + end + + test "omitting amount settles the occurrence in full" do + result = call_tool({}) + + assert result[:recorded] + assert_equal "paid", result[:occurrence][:status] + assert @occurrence.reload.paid? + assert_equal 2000, @occurrence.allocations.sum(:allocated_amount) + end + + test "a backdated full settlement carries its payment date into the allocation" do + paid_on = Date.current - 6 + + result = call_tool("paid_on" => paid_on.iso8601) + + assert result[:recorded] + assert_equal paid_on, @occurrence.reload.allocations.sole.paid_on, + "the settlement must record the stated payment date, not today" + end + + test "a partial payment leaves the occurrence open and partially paid" do + result = call_tool("amount" => 500) + + assert result[:recorded] + occurrence = @occurrence.reload + assert occurrence.scheduled?, "500 against 2000 is not rent" + assert occurrence.partially_paid? + assert_equal "$1,500.00", result[:occurrence][:remaining] + end + + test "an invalid payment amount is refused with a hint, not raised" do + result = call_tool("amount" => 0) + + assert result[:error].present? + assert_includes result[:hint], "get_bill_details" + assert_equal 0, @occurrence.reload.allocations.count + end + + test "after settling, the next open occurrence becomes the payable one" do + call_tool({}) + + result = call_tool("amount" => 100) + + assert result[:recorded], "the series' next open occurrence takes the payment" + next_open = @series.recurring_occurrences.open_status.order(:due_on).first + assert next_open.partially_paid? + end + + test "a closed or unknown due date lists the open ones" do + result = call_tool("occurrence_due_on" => (Date.current - 3).iso8601) + + assert_match(/No open occurrence/, result[:error]) + assert_includes result[:hint], @occurrence.due_on.iso8601 + end + + test "payments go through the Allocator write path" do + result = call_tool("amount" => 500, "paid_on" => (Date.current - 1).iso8601) + + assert result[:recorded] + allocation = @occurrence.allocations.sole + assert_equal "user_created", allocation.source + assert_equal Date.current - 1, allocation.paid_on + end + + + # An LLM retries on timeout. Every payment recorded through this tool was + # capped by nothing, because the allocator only guarded payments attached to + # a bank entry. Two identical calls settled a $2,000 bill at $3,000. + test "a repeated partial payment cannot overfill the cycle" do + series = declare_capped_bill(amount: 2000) + args = { "bill_id" => series.id, "amount" => 1500 } + + first = Assistant::Function::RecordBillPayment.new(@user).call(args) + second = Assistant::Function::RecordBillPayment.new(@user).call(args) + + assert first[:recorded] + assert second[:error].present?, "the retry must be refused, not absorbed" + assert_match(/more than the/, second[:error]) + + occurrence = series.recurring_occurrences.order(:due_on).first + assert_equal 1500, occurrence.allocations.confirmed.sum(:allocated_amount) + end + + test "an amount larger than the cycle owes is refused" do + series = declare_capped_bill(amount: 2000) + + result = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id, "amount" => 999_999) + + assert result[:error].present? + assert_equal 0, series.recurring_occurrences.order(:due_on).first.allocations.count + end + + # After a cycle settles, the next one becomes current. A retried settle used + # to fall through and pay it, closing two cycles from two identical requests. + test "a repeated full settle does not pay the following cycle" do + series = declare_capped_bill(amount: 2000) + + first = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id) + second = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id) + + assert first[:recorded] + assert second[:error].present?, "nothing is owed once the due cycle is settled" + assert_match(/pay ahead/, second[:hint]) + assert_equal 1, series.recurring_occurrences.where(status: "paid").count + end + + test "paying ahead still works when the cycle is named" do + series = declare_capped_bill(amount: 2000) + Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id) + future = series.recurring_occurrences.open_status.order(:due_on).first + + result = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id, "occurrence_due_on" => future.due_on.iso8601) + + assert result[:recorded], "naming the date is the deliberate act a retry never performs" + end + + test "non-finite and negative amounts are refused instead of coerced" do + series = declare_capped_bill(amount: 2000) + + %w[Infinity NaN].each do |value| + result = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id, "amount" => value) + assert_equal "amount is not a number", result[:error] + end + + negative = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id, "amount" => -500) + assert_equal "amount must be greater than zero", negative[:error], + "a negative used to be flipped with .abs and recorded as a payment" + + assert_equal 0, series.recurring_occurrences.order(:due_on).first.allocations.count + end + + # "" is not "no amount": a present-but-blank value used to skip the amount + # branch entirely and settle the whole occurrence. + test "a blank amount is malformed input, not a settlement" do + series = declare_capped_bill(amount: 100) + + result = Assistant::Function::RecordBillPayment.new(@user).call("bill_id" => series.id, "amount" => "") + + assert_match(/not a number/, result[:error]) + assert_equal 0, series.recurring_occurrences.order(:due_on).first.allocations.count + end + + test "a read-only account share cannot record payments" do + series = declare_capped_bill(amount: 100) + series.update!(account: accounts(:credit_card)) + member = users(:family_member) + + result = Assistant::Function::RecordBillPayment.new(member).call("bill_id" => series.id) + + assert_match(/read-only/, result[:error]) + assert_equal 0, series.recurring_occurrences.order(:due_on).first.allocations.count + end + + private + + def declare_capped_bill(amount:) + due = Date.current + series = @family.recurring_transactions.create!( + name: "Rent #{amount}", account: accounts(:depository), amount: amount, + currency: "USD", status: "active", bill_type: "bill", manual: true, + dedup_scope: "rent-#{amount}", expected_day_of_month: due.day, + last_occurrence_date: 1.month.ago.to_date, next_expected_date: due + ) + series.recurring_occurrences.destroy_all + series.recurring_occurrences.create!( + family: @family, original_due_on: due, due_on: due, + currency: "USD", expected_amount: amount, status: "scheduled" + ) + series.recurring_occurrences.create!( + family: @family, original_due_on: due + 30, due_on: due + 30, + currency: "USD", expected_amount: amount, status: "scheduled" + ) + series.reload + end + + def call_tool(params) + Assistant::Function::RecordBillPayment.new(@user).call({ "bill_id" => @series.id.to_s }.merge(params)) + end +end diff --git a/test/models/assistant/function/update_bill_test.rb b/test/models/assistant/function/update_bill_test.rb new file mode 100644 index 000000000..cf5226c9d --- /dev/null +++ b/test/models/assistant/function/update_bill_test.rb @@ -0,0 +1,203 @@ +require "test_helper" + +class Assistant::Function::UpdateBillTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @family = @user.family + @family.recurring_transactions.destroy_all + end + + test "amount arrives as magnitude and income keeps its sign" do + income = create_series(name: "Paycheck", amount: -1200, bill_type: "income") + + result = call_tool(income.id, "amount" => 1300) + + assert result[:updated] + assert_equal(-1300, income.reload.amount.to_f, + "a raw assignment would have flipped the paycheck into a bill") + end + + test "a non-numeric amount is refused with a hint, not raised" do + series = create_series(name: "Gym", amount: 40) + + result = call_tool(series.id, "amount" => "twenty") + + assert result[:error].present? + assert result[:hint].present? + assert_equal 40, series.reload.amount.to_f + end + + test "a non-finite amount is refused, not assigned" do + # BigDecimal happily parses these strings, so the finite check is the + # only thing between them and the ledger. + series = create_series(name: "Gym", amount: 40) + + %w[Infinity -Infinity NaN].each do |value| + result = call_tool(series.id, "amount" => value) + + assert result[:error].present?, "#{value} must be refused" + assert_equal 40, series.reload.amount.to_f + end + end + + test "a schedule change applies the preset and pins it against detection" do + series = create_series(name: "Gym", amount: 40) + refute series.schedule_pinned? + + result = call_tool(series.id, "frequency" => "weekly", "weekday" => 5) + + assert result[:updated] + assert_includes result[:changed_fields], "schedule" + assert series.reload.schedule_pinned?, + "an AI-applied cadence is user intent by proxy; detection must not move it back" + end + + test "paused maps to the stored value the UI's own Pause writes" do + series = create_series(name: "Gym", amount: 40) + + result = call_tool(series.id, "status" => "paused") + + assert result[:updated] + assert_equal "inactive", series.reload.status + assert_equal "paused", result[:bill][:status], "but the tool answers in the UI vocabulary" + end + + test "an amount change pins occurrences already due instead of restating them" do + overdue_day = 10.days.ago.to_date + series = create_series(name: "Rent", amount: 2000, + expected_day_of_month: overdue_day.day, + anchor_date: overdue_day, + last_occurrence_date: overdue_day << 1, + next_expected_date: overdue_day) + past_due = series.recurring_occurrences.open_status.find_by!(due_on: overdue_day) + + call_tool(series.id, "amount" => 2500) + + assert_equal 2000, past_due.reload.resolved_expected_amount.to_f, + "raising the rent must not restate what last month's unpaid rent claims" + end + + test "an invalid renews_on is rejected instead of silently clearing" do + series = create_series(name: "Gym", amount: 40) + + result = call_tool(series.id, "renews_on" => "not-a-date") + + assert_equal "renews_on is not a valid date", result[:error] + assert_equal "Use YYYY-MM-DD.", result[:hint] + assert_nil series.reload.renews_on + + result = call_tool(series.id, "renews_on" => "2027-03-01") + + assert result[:updated] + assert_equal Date.new(2027, 3, 1), series.reload.renews_on + end + + test "kind cannot leave the editable set" do + income = create_series(name: "Paycheck", amount: -1200, bill_type: "income") + + result = call_tool(income.id, "bill_type" => "bill") + + assert result[:error].present? + assert_equal "income", income.reload.bill_type + end + + test "an inaccessible account name returns a hint" do + series = create_series(name: "Gym", amount: 40) + + result = call_tool(series.id, "account_name" => "No Such Account") + + assert_match(/No account named/, result[:error]) + assert_includes result[:hint], "get_accounts" + end + + test "no recognized fields is an error, not a silent no-op" do + series = create_series(name: "Gym", amount: 40) + + result = call_tool(series.id, "unknown_field" => "x") + + assert_match(/No recognized fields/, result[:error]) + end + + # Sharing is per account: a read-only share reads a bill everywhere the app + # shows it and must not change it, mirroring the pages' write guard. + test "a read-only account share cannot update the series" do + series = create_series(name: "Shared Sub", amount: 12, account: accounts(:credit_card)) + member = users(:family_member) + + result = Assistant::Function::UpdateBill.new(member).call("bill_id" => series.id.to_s, "name" => "Hijacked") + + assert_match(/read-only/, result[:error]) + assert_equal "Shared Sub", series.reload.name + end + + test "the reassignment target must be writable" do + series = create_series(name: "Wandering", amount: 9, account: nil) + member = users(:family_member) + + result = Assistant::Function::UpdateBill.new(member).call( + "bill_id" => series.id.to_s, "account_name" => accounts(:credit_card).name + ) + + assert_match(/add bills to/, result[:error]) + assert_nil series.reload.account_id + end + + test "namesake accounts are refused rather than picked between" do + series = create_series(name: "Gym", amount: 40) + @family.accounts.create!( + name: accounts(:depository).name, balance: 0, currency: "USD", + accountable: Depository.new, owner: @user + ) + + result = call_tool(series.id, "account_name" => accounts(:depository).name) + + assert_match(/More than one account/, result[:error]) + end + + test "an unrecognized frequency is rejected before anything saves" do + series = create_series(name: "Insurance", amount: 60) + + result = call_tool(series.id, "frequency" => "yearly", "name" => "Renamed") + + assert_match(/not a frequency/, result[:error]) + assert_equal "Insurance", series.reload.name, "companion edits must not save with a rejected cadence" + end + + test "day details without a frequency are refused, not silently dropped" do + series = create_series(name: "Rent", amount: 2150) + + result = call_tool(series.id, "due_day_of_month" => 5) + + assert_match(/need frequency/, result[:error]) + end + + test "unrecognized status and kind values error instead of no-opping" do + series = create_series(name: "Gym", amount: 40) + + assert_match(/not a status/, call_tool(series.id, "status" => "cancelled")[:error]) + assert_match(/not a kind/, call_tool(series.id, "bill_type" => "loan")[:error]) + end + + private + + def call_tool(bill_id, params) + Assistant::Function::UpdateBill.new(@user).call({ "bill_id" => bill_id.to_s }.merge(params)) + end + + def create_series(name:, amount:, **overrides) + @family.recurring_transactions.create!({ + name: name, + account: accounts(:depository), + amount: amount, + dedup_scope: "#{name}-#{amount}", + currency: "USD", + expected_day_of_month: Date.current.day, + anchor_date: Date.current, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: Date.current, + status: "active", + manual: true, + bill_type: amount.to_d.negative? ? "income" : "bill" + }.merge(overrides)) + end +end diff --git a/test/models/assistant/responder_test.rb b/test/models/assistant/responder_test.rb index 372afddf0..c3f1d9022 100644 --- a/test/models/assistant/responder_test.rb +++ b/test/models/assistant/responder_test.rb @@ -70,6 +70,21 @@ class Assistant::ResponderTest < ActiveSupport::TestCase assert tool_choices_seen[0..-2].all?(&:nil?) end + test "a model that never stops calling tools hits the cap and raises" 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 ] + ) + + @llm.stubs(:chat_response).returns(provider_success_response(tool_response)) + + with_iteration_cap(2) do + assert_raises(Assistant::Responder::ToolCallLimitError) { @responder.respond } + end + end + private def with_iteration_cap(value) previous = ENV["ASSISTANT_MAX_TOOL_CALL_ITERATIONS"] diff --git a/test/models/recurring_transaction/allocator_test.rb b/test/models/recurring_transaction/allocator_test.rb index f2804aae5..eacbd674a 100644 --- a/test/models/recurring_transaction/allocator_test.rb +++ b/test/models/recurring_transaction/allocator_test.rb @@ -295,6 +295,36 @@ class RecurringTransaction::AllocatorTest < ActiveSupport::TestCase assert_nil allocation.source_amount end + # The remainder guard is opt-in and lives INSIDE the occurrence lock, + # because a caller's pre-lock capacity check is advisory: two concurrent + # payments can both read the same stale remainder. Sequentially that + # surfaces as: the second payment sees the first one's allocation. + test "a capped amount cannot exceed what remains on the cycle" do + occurrence = usd_occurrence(expected: 100) + allocator = RecurringTransaction::Allocator.new(occurrence) + + allocator.allocate!(amount: 60, source: "user_created", cap_at_remaining: true) + + error = assert_raises(RecurringTransaction::Allocator::OverAllocationError) do + allocator.allocate!(amount: 60, source: "user_created", cap_at_remaining: true) + end + assert_match(/remaining/, error.message) + assert_equal 60, occurrence.reload.allocations.sum(:allocated_amount) + end + + # Exceeding the remainder stays legal for every caller that does not ask + # for capping: a single settlement above the expected amount is how a price + # rise gets recorded, and PriceChangeDetector reads exactly those. + test "an uncapped settlement above the expected amount is still permitted" do + occurrence = usd_occurrence(expected: 80) + + allocation = RecurringTransaction::Allocator.new(occurrence) + .allocate!(amount: 95, source: "user_created") + + assert allocation.persisted? + assert_equal 95, occurrence.reload.allocations.sum(:allocated_amount) + end + private def foreign_entry(amount:, currency:)