From ce92b363515760a33483ccbaf44244cc0eeff96c Mon Sep 17 00:00:00 2001 From: Brandon Date: Tue, 1 Sep 2026 22:06:13 -0700 Subject: [PATCH] feat(bills): assistant and MCP tools for bills (#3203) * feat(bills): assistant and MCP tools for bills Last of three chunks carved out of #3083, stacked on the UI bundle. Exposes bills to the builtin assistant and to MCP clients. Everything here is gated behind preview features, so the tools are absent from tools/list until a user opts in. Seven tools: - get_bills, get_bill_details and get_paycheck_plan for reads - get_bill_audit, a deterministic review that surfaces likely duplicates, price changes, trials about to convert, upcoming renewals and long-overdue bills - create_bill, update_bill and record_bill_payment for writes Shared argument parsing, permission checks and error shapes live in BillsSupport, so every tool answers with the same {error, hint} contract the existing tools use, and a bad argument never aborts the turn. The write tools mutate financial records on a model's say-so, so they refuse rather than guess: a payment cannot exceed what its cycle still owes, a repeated settle will not quietly close next month, an unrecognized frequency is an error instead of a silent monthly default, and non-finite or negative amounts are rejected before they reach the database. The read tools say what they filtered. An empty result names the statuses that do hold matches, the paycheck plan discloses the unconfirmed series it excluded from spending headroom, and history and price-change windows report their real totals rather than letting a caller sum a truncated list. A not-found no longer returns the scoped relation's SQL, which handed any MCP client the access-control schema for the cost of a guessed id. The in-page AI helpers are not here. Smart fill and smart configuration are buttons on the bills pages, so they ship with the UI bundle along with the provider-side suggester they call. Suite 7,854 runs, 0 failures. Rubocop clean, eager loading verified. * Address the ready-review round * Reject an out-of-range audit lookback out loud * Speak the cycle remainder guard through the allocator locale --- app/controllers/mcp_controller.rb | 9 +- app/models/assistant.rb | 20 +- app/models/assistant/configurable.rb | 1 + .../assistant/function/bills_support.rb | 158 ++++++++++ app/models/assistant/function/create_bill.rb | 190 ++++++++++++ .../assistant/function/get_bill_audit.rb | 182 ++++++++++++ .../assistant/function/get_bill_details.rb | 169 +++++++++++ app/models/assistant/function/get_bills.rb | 219 ++++++++++++++ .../assistant/function/get_paycheck_plan.rb | 159 ++++++++++ .../assistant/function/record_bill_payment.rb | 184 ++++++++++++ app/models/assistant/function/update_bill.rb | 274 ++++++++++++++++++ app/models/assistant/function_tool_caller.rb | 16 +- app/models/recurring_transaction/allocator.rb | 14 +- .../models/recurring_transaction/en.yml | 1 + docs/hosting/mcp.md | 16 +- test/controllers/mcp_controller_test.rb | 9 +- test/models/assistant/configurable_test.rb | 17 ++ .../function/bills_tools_schema_test.rb | 74 +++++ .../assistant/function/create_bill_test.rb | 170 +++++++++++ .../assistant/function/get_bill_audit_test.rb | 133 +++++++++ .../function/get_bill_details_test.rb | 146 ++++++++++ .../assistant/function/get_bills_test.rb | 240 +++++++++++++++ .../function/get_paycheck_plan_test.rb | 194 +++++++++++++ .../function/record_bill_payment_test.rb | 194 +++++++++++++ .../assistant/function/update_bill_test.rb | 203 +++++++++++++ test/models/assistant/responder_test.rb | 15 + .../recurring_transaction/allocator_test.rb | 30 ++ 27 files changed, 3026 insertions(+), 11 deletions(-) create mode 100644 app/models/assistant/function/bills_support.rb create mode 100644 app/models/assistant/function/create_bill.rb create mode 100644 app/models/assistant/function/get_bill_audit.rb create mode 100644 app/models/assistant/function/get_bill_details.rb create mode 100644 app/models/assistant/function/get_bills.rb create mode 100644 app/models/assistant/function/get_paycheck_plan.rb create mode 100644 app/models/assistant/function/record_bill_payment.rb create mode 100644 app/models/assistant/function/update_bill.rb create mode 100644 test/models/assistant/function/bills_tools_schema_test.rb create mode 100644 test/models/assistant/function/create_bill_test.rb create mode 100644 test/models/assistant/function/get_bill_audit_test.rb create mode 100644 test/models/assistant/function/get_bill_details_test.rb create mode 100644 test/models/assistant/function/get_bills_test.rb create mode 100644 test/models/assistant/function/get_paycheck_plan_test.rb create mode 100644 test/models/assistant/function/record_bill_payment_test.rb create mode 100644 test/models/assistant/function/update_bill_test.rb 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:)